From: Arsh Srivastava Date: Fri, 13 Mar 2026 03:13:14 GMT Subject: Re: [PATCH v7] unpack-trees: suggest using 'git stash' when checkout fails Message-ID: In-Reply-To: Junio C Hamano writes : > The first paragraph is a bit of a run-on and has a misplaced "and"; > I cannot quite read and understand this overly long single sentence. > Perhaps the early part can become a bit easier to read with punctuations, and cutting the sentence into two, e.g., In my future commits I will remember to make it as easy to read as possible. With less punctuations and shorter sentences which will in turn make it more concise. > Also it is misleading to say "previous" error message. We talk > about the current code in the present tense, to highlight what the > problem in the current code is. Understood I will in future not use previous because it is the _current_ code. > You may view it as a weakness (which > may motivate this patch to be written). But I personally am not so > sure that adding words to the existing message would necessarily > make it more clear. I understand, git wants people to not explore the available change options and help them make logical decisions rather than pushing them with some unneeded commands. > As Documentation/SubmittingPatches says, let's instruct the code to > "be like so" in imperative mood. E.g., "Enhance the error > message..." instead of "This patch enhances...". Understood that makes sense because nevertheless it is given that I am writing the changes for this patch only. > These were already overly long, but the updated one is way too long > to be read on end-user's terminal. The source lines are overly > long, too. That makes total sense. > to those users who decline the advice, we now show "Please > commit...". That is not what !advice_enabled() should trigger, is > it? Thank you so much for your guidance the advice should not trigger to those who have opted not to see. My code might have misjudged this paradigm. > Also "To move you" -> "To move your". I thought I had fixed this typo. Seems like I didn't. I will remember to be more cautious next time. > Also the advice lost the other possiblity of first committing the > work in progress on the original branch before switching, yet the > new advice message is quite wordy. Absolutely correct this commit does narrow the users vision for exploring. > Also, using "for safe merge" when the user is performing a > "checkout" might be slightly confusing, even if 'stash pop' involves > a merge under the hood. I don't want to sound like a programmed robot but I absolutely agree with the recommendations. > But as I already said, I think the current text may already strike > the right balance between being clear and being concise. Thank you so much for your valuable guidance. If it's possible I want some guidance over the questions written below, As it is well stated by you that the current text is clear enough. Should I still work on this PR from a purely GSoC perspective. Or should I start making my proposal or still work on this PR until my micro project is merged? Because I have already shown I can navigate git project which was the goal of micro projects in the first place. On Fri, 13 Mar 2026 at 04:10, Junio C Hamano wrote: > > "Arsh Srivastava via GitGitGadget" writes: > > > When a branch switch fails due to local changes and > > new users who are not familiar with the error message often > > get confused about how to move ahead and resolve the issue as > > the previous error message only suggests to commit or stash the changes > > but doesn't explain how to do that or what the next steps are. > > The first paragraph is a bit of a run-on and has a misplaced "and"; > I cannot quite read and understand this overly long single sentence. > > Perhaps the early part can become a bit easier to read with > punctuations, and cutting the sentence into two, e.g., > > When a branch switch fails due to local changes, new users who > are unfamiliar with the error message often get confused about how > to move ahead and resolve the issue. > > Also it is misleading to say "previous" error message. We talk > about the current code in the present tense, to highlight what the > problem in the current code is. The _current_ message stops at > hinting the commands to be used without giving wordy instructions > that are best left to manuals. You may view it as a weakness (which > may motivate this patch to be written). But I personally am not so > sure that adding words to the existing message would necessarily > make it more clear. > > > This patch enhances the error message with more specific > > instructions in a concise manner to help users understand > > how to resolve the issue and move their local changes > > safely to the other branch using stash. > > As Documentation/SubmittingPatches says, let's instruct the code to > "be like so" in imperative mood. E.g., "Enhance the error > message..." instead of "This patch enhances...". > > By the way, the updated message seems much less concise than the > original. > > > msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE) > > ? _("Your local changes to the following files would be overwritten by checkout:\n%%s" > > - "Please commit your changes or stash them before you switch branches.") > > - : _("Your local changes to the following files would be overwritten by checkout:\n%%s"); > > + "To move you local changes safely to the other branch,\n" > > + "Please try 'git stash' followed by 'git checkout ' followed by 'git stash pop' for safe merge." > > + ) > > + : _("Your local changes to the following files would be overwritten by checkout:\n%%s" > > + "Please commit your changes or stash them before you switch branches."); > > These were already overly long, but the updated one is way too long > to be read on end-user's terminal. The source lines are overly > long, too. > > The original was this: > > msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE) > ? _("Your local changes to the following files would be overwritten by checkout:\n%%s" > "Please commit your changes or stash them before you switch branches.") > : _("Your local changes to the following files would be overwritten by checkout:\n%%s"); > > Note that when advice is *NOT* enabled, we only gave > > _("Your local changes to the following files would be overwritten by checkout:\n%%s"); > > without any "advise" in the output. That is what !advice_enabled() means. > > The updated code does this: > > msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE) > ? _("Your local changes to the following files would be overwritten by checkout:\n%%s" > "To move you local changes safely to the other branch,\n" > "Please try 'git stash' followed by 'git checkout ' followed by 'git stash pop' for safe merge." > ) > : _("Your local changes to the following files would be overwritten by checkout:\n%%s" > "Please commit your changes or stash them before you switch branches."); > > to those users who decline the advice, we now show "Please > commit...". That is not what !advice_enabled() should trigger, is > it? > > Also "To move you" -> "To move your". > > Also the advice lost the other possiblity of first committing the > work in progress on the original branch before switching, yet the > new advice message is quite wordy. > > Also, using "for safe merge" when the user is performing a > "checkout" might be slightly confusing, even if 'stash pop' involves > a merge under the hood. > > A more concise version might say: > > Try 'git stash && git checkout && git stash pop' to carry > your changes to the new branch, or commit your work before switching. > > But as I already said, I think the current text may already strike > the right balance between being clear and being concise. > > Thanks. >