Advice on checkout dirty files#2233
Open
Arsh123344423 wants to merge 1 commit intogit:masterfrom
Open
Conversation
9040583 to
9ec447e
Compare
Author
Author
|
/submit |
|
Submitted as pull.2233.git.git.1773132678.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Phillip Wood wrote on the Git mailing list (how to reply to this email): Hi Arsh
On 10/03/2026 08:51, Arsh Srivastava via GitGitGadget wrote:
> This is my submission for microproject [GSOC]
> > This patch adds a new advice type ADVICE_STASH_BEFORE_CHECKOUT to help users
> when they attempt to switch branches with local modifications that would be
> overwritten by the operation.
If the intent is for the user to carry over the changes to the new branch then recommending "git checkout -m" might be more convenient rather than having to stash, checkout and unstash as three separate steps.
Something seems to have gone awry with your branch as there are other patches in this series. You should rebase your branch onto the upstream master branch with
git rebase --onto origin/master HEAD^
and then when you push it double check how many commits there are in the summary of the pull request before submitting.
Thanks
Phillip
> The new advice follows the same patterns established by existing advice
> functions such as advise_on_updating_sparse_paths(). When triggered, it
> lists the affected files and suggests using git stash push/pop to save and
> restore local changes.
> > The advice can be silenced with:
> > git config set advice.stashBeforeCheckout false
> > Changes:
> >> advice.h: add ADVICE_STASH_BEFORE_CHECKOUT enum value advice.c: add
>> "stashBeforeCheckout" to advice_setting[] and implement
>> advise_on_checkout_dirty_files() function
>> Documentation/config/advice.adoc: document the new advice key
> > Signed-off-by: Arsh Srivastava arshsrivastava00@gmail.com
> > Arsh Srivastava (1):
> advice: add stashBeforeCheckout advice for dirty branch switches
> > Junio C Hamano (1):
> The 13th batch
> > K Jayatheerth (1):
> repo: remove unnecessary variable shadow
> > LorenzoPegorari (2):
> diff: handle ANSI escape codes in prefix when calculating diffstat
> width
> t4052: test for diffstat width when prefix contains ANSI escape codes
> > Documentation/RelNotes/2.54.0.adoc | 14 +++++++++++++
> Documentation/config/advice.adoc | 5 +++++
> advice.c | 27 +++++++++++++++++++++++++
> advice.h | 2 ++
> builtin/repo.c | 1 -
> diff.c | 12 ++++-------
> t/t4052-stat-output.sh | 32 ++++++++++++++++++++++++++++++
> 7 files changed, 84 insertions(+), 9 deletions(-)
> > > base-commit: 3fe08b8fd1f7731edabeab8138547ec88d6407de
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2233%2FArsh123344423%2Fadvice_on_checkout_dirty_files-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2233/Arsh123344423/advice_on_checkout_dirty_files-v1
> Pull-Request: https://github.com/git/git/pull/2233 |
|
User |
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Thank you so much for the advice I will
> rebase my file’s pointer head
And just wanted to know what do you recommend should be my changed
approach for me over this pr to make that function useful
On Tue, 10 Mar 2026 at 4:03 PM, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Arsh
>
> On 10/03/2026 08:51, Arsh Srivastava via GitGitGadget wrote:
> > This is my submission for microproject [GSOC]
> >
> > This patch adds a new advice type ADVICE_STASH_BEFORE_CHECKOUT to help users
> > when they attempt to switch branches with local modifications that would be
> > overwritten by the operation.
>
> If the intent is for the user to carry over the changes to the new
> branch then recommending "git checkout -m" might be more convenient
> rather than having to stash, checkout and unstash as three separate steps.
>
> Something seems to have gone awry with your branch as there are other
> patches in this series. You should rebase your branch onto the upstream
> master branch with
>
> git rebase --onto origin/master HEAD^
>
> and then when you push it double check how many commits there are in the
> summary of the pull request before submitting.
>
> Thanks
>
> Phillip
>
> > The new advice follows the same patterns established by existing advice
> > functions such as advise_on_updating_sparse_paths(). When triggered, it
> > lists the affected files and suggests using git stash push/pop to save and
> > restore local changes.
> >
> > The advice can be silenced with:
> >
> > git config set advice.stashBeforeCheckout false
> >
> > Changes:
> >
> >> advice.h: add ADVICE_STASH_BEFORE_CHECKOUT enum value advice.c: add
> >> "stashBeforeCheckout" to advice_setting[] and implement
> >> advise_on_checkout_dirty_files() function
> >> Documentation/config/advice.adoc: document the new advice key
> >
> > Signed-off-by: Arsh Srivastava arshsrivastava00@gmail.com
> >
> > Arsh Srivastava (1):
> > advice: add stashBeforeCheckout advice for dirty branch switches
> >
> > Junio C Hamano (1):
> > The 13th batch
> >
> > K Jayatheerth (1):
> > repo: remove unnecessary variable shadow
> >
> > LorenzoPegorari (2):
> > diff: handle ANSI escape codes in prefix when calculating diffstat
> > width
> > t4052: test for diffstat width when prefix contains ANSI escape codes
> >
> > Documentation/RelNotes/2.54.0.adoc | 14 +++++++++++++
> > Documentation/config/advice.adoc | 5 +++++
> > advice.c | 27 +++++++++++++++++++++++++
> > advice.h | 2 ++
> > builtin/repo.c | 1 -
> > diff.c | 12 ++++-------
> > t/t4052-stat-output.sh | 32 ++++++++++++++++++++++++++++++
> > 7 files changed, 84 insertions(+), 9 deletions(-)
> >
> >
> > base-commit: 3fe08b8fd1f7731edabeab8138547ec88d6407de
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2233%2FArsh123344423%2Fadvice_on_checkout_dirty_files-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2233/Arsh123344423/advice_on_checkout_dirty_files-v1
> > Pull-Request: https://github.com/git/git/pull/2233
> |
|
User |
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Hi,
Thank you for the feedback!
That's a great point. Using "git checkout -m" is indeed more convenient
when the user wants to carry their changes over to the new branch, as it
merges the local modifications into the new branch in a single step
rather than requiring stash, checkout, and unstash separately.
I will update the advice message to mention "git checkout -m" as the
primary suggestion when the intent is to carry changes over, and keep
git stash push/pop as an alternative for when the user wants to
temporarily set changes aside.
Will send a v2 shortly.
Signed-off-by: Arsh Srivastava <arshsrivastava00@gmail.com> |
9ec447e to
eb5639d
Compare
Author
|
/submit |
|
Submitted as pull.2233.v2.git.git.1773140364525.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag
8000
|
|
There is an issue in commit e644634:
|
e644634 to
e88c851
Compare
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): I have as you mentioned changed
> Rebased my files pointer
> Changed advice to git checkout -m
Thank you
Signed-off-by: Arsh Srivastava <arshsrivastava00@gmail.com>
On Tue, 10 Mar 2026 at 16:33, Arsh Srivastava
<arshsrivastava00@gmail.com> wrote:
>
> I have as you mentioned changed
> > Rebased my files pointer
> > Changed advice to git checkout -m
> Thank you
>
> Signed-off-by: Arsh Srivastava <arshsrivastava00@gmail.com>
>
> On Tue, 10 Mar 2026 at 4:29 PM, Arsh Srivastava via GitGitGadget <gitgitgadget@gmail.com> wrote:
>>
>> From: Arsh Srivastava <arshsrivastava00@gmail.com>
>>
>> Add a new advice type ADVICE_STASH_BEFORE_CHECKOUT to guide users
>> when they attempt to switch branches with local modifications that
>> would be overwritten by the operation.
>>
>> This includes:
>> > New ADVICE_STASH_BEFORE_CHECKOUT enum value in advice.h
>> > Corresponding "stashBeforeCheckout" entry in advice_setting[]
>> > New advise_on_checkout_dirty_files() function that lists the
>> affected files and suggests using git stash push/pop
>> > Documentation entry in Documentation/config/advice.txt
>>
>> The advice follows existing patterns established by
>> advise_on_updating_sparse_paths() and can be silenced with:
>>
>> git config set advice.stashBeforeCheckout false
>>
>> Signed-off-by: Arsh Srivastava <arshsrivastava00@gmail.com>
>> ---
>> Advice on checkout dirty files
>>
>> This is my submission for microproject [GSOC]
>>
>> This patch adds a new advice type ADVICE_STASH_BEFORE_CHECKOUT to help
>> users when they attempt to switch branches with local modifications that
>> would be overwritten by the operation.
>>
>> The new advice follows the same patterns established by existing advice
>> functions such as advise_on_updating_sparse_paths(). When triggered, it
>> lists the affected files and suggests using git stash push/pop to save
>> and restore local changes.
>>
>> The advice can be silenced with:
>>
>> git config set advice.stashBeforeCheckout false
>>
>> Changes:
>>
>> > advice.h: add ADVICE_STASH_BEFORE_CHECKOUT enum value advice.c: add
>> > "stashBeforeCheckout" to advice_setting[] and implement
>> > advise_on_checkout_dirty_files() function
>> > Documentation/config/advice.adoc: document the new advice key
>>
>> Signed-off-by: Arsh Srivastava arshsrivastava00@gmail.com
>>
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2233%2FArsh123344423%2Fadvice_on_checkout_dirty_files-v2
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2233/Arsh123344423/advice_on_checkout_dirty_files-v2
>> Pull-Request: https://github.com/git/git/pull/2233
>>
>> Range-diff vs v1:
>>
>> 1: 0ed992956e < -: ---------- diff: handle ANSI escape codes in prefix when calculating diffstat width
>> 2: c70043a2c0 < -: ---------- t4052: test for diffstat width when prefix contains ANSI escape codes
>> 3: 185356a454 < -: ---------- repo: remove unnecessary variable shadow
>> 4: acebdd714b < -: ---------- The 13th batch
>> 5: 9ec447e3cb = 1: eb5639dbc3 advice: add stashBeforeCheckout advice for dirty branch switches
>>
>>
>> Documentation/config/advice.adoc | 5 +++++
>> advice.c | 27 +++++++++++++++++++++++++++
>> advice.h | 2 ++
>> 3 files changed, 34 insertions(+)
>>
>> diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
>> index 257db58918..8752e05636 100644
>> --- a/Documentation/config/advice.adoc
>> +++ b/Documentation/config/advice.adoc
>> @@ -126,6 +126,11 @@ all advice messages.
>> Shown when a sparse index is expanded to a full index, which is likely
>> due to an unexpected set of files existing outside of the
>> sparse-checkout.
>> + stashBeforeCheckout::
>> + Shown when the user attempts to switch branches but has
>> + local modifications that would be overwritten by the
>> + operation, to suggest using linkgit:git-stash[1] to
>> + save changes before switching.
>> statusAheadBehind::
>> Shown when linkgit:git-status[1] computes the ahead/behind
>> counts for a local ref compared to its remote tracking ref,
>> diff --git a/advice.c b/advice.c
>> index 0018501b7b..e1264f525c 100644
>> --- a/advice.c
>> +++ b/advice.c
>> @@ -81,6 +81,7 @@ static struct {
>> [ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure" },
>> [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks" },
>> [ADVICE_SPARSE_INDEX_EXPANDED] = { "sparseIndexExpanded" },
>> + [ADVICE_STASH_BEFORE_CHECKOUT] = { "stashBeforeCheckout" },
>> [ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning" },
>> [ADVICE_STATUS_HINTS] = { "statusHints" },
>> [ADVICE_STATUS_U_OPTION] = { "statusUoption" },
>> @@ -312,3 +313,29 @@ void advise_on_moving_dirty_path(struct string_list *pathspec_list)
>> "* Use \"git add --sparse <paths>\" to update the index\n"
>> "* Use \"git sparse-checkout reapply\" to apply the sparsity rules"));
>> }
>> +
>> +void advise_on_checkout_dirty_files(struct string_list *file_list)
>> +{
>> + struct string_list_item *item;
>> +
>> + if (!file_list->nr)
>> + return;
>> +
>> + fprintf(stderr, _("The following files have local modifications that would\n"
>> + "be overwritten by switching branches:\n"));
>> + for_each_string_list_item(item, file_list)
>> + fprintf(stderr, "\t%s\n", item->string);
>> +
>> + advise_if_enabled(ADVICE_STASH_BEFORE_CHECKOUT,
>> + _("You can save your local changes before switching by running:\n"
>> + "\n"
>> + "\tgit stash push\n"
>> + "\n"
>> + "Then restore them after switching with:\n"
>> + "\n"
>> + "\tgit stash pop\n"
>> + "\n"
>> + "Or to discard your local changes, use:\n"
>> + "\n"
>> + "\tgit checkout -- <file>"));
>> +}
>> diff --git a/advice.h b/advice.h
>> index 8def280688..c035b5d8e3 100644
>> --- a/advice.h
>> +++ b/advice.h
>> @@ -48,6 +48,7 @@ enum advice_type {
>> ADVICE_SET_UPSTREAM_FAILURE,
>> ADVICE_SKIPPED_CHERRY_PICKS,
>> ADVICE_SPARSE_INDEX_EXPANDED,
>> + ADVICE_STASH_BEFORE_CHECKOUT,
>> ADVICE_STATUS_AHEAD_BEHIND_WARNING,
>> ADVICE_STATUS_HINTS,
>> ADVICE_STATUS_U_OPTION,
>> @@ -83,5 +84,6 @@ void NORETURN die_ff_impossible(void);
>> void advise_on_updating_sparse_paths(struct string_list *pathspec_list);
>> void detach_advice(const char *new_name);
>> void advise_on_moving_dirty_path(struct string_list *pathspec_list);
>> +void advise_on_checkout_dirty_files(struct string_list *file_list);
>>
>> #endif /* ADVICE_H */
>>
>> base-commit: d181b9354cf85b44455ce3ca9e6af0b9559e0ae2
>> --
>> gitgitgadget |
Author
|
/preview |
|
Preview email sent as pull.2233.v3.git.git.1773143639.gitgitgadget@gmail.com |
Author
|
/submit |
|
Submitted as pull.2233.v3.git.git.1773149337.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Patrick Steinhardt wrote on the Git mailing list (how to reply to this email): On Tue, Mar 10, 2026 at 10:59:24AM +0000, Arsh Srivastava via GitGitGadget wrote:
> diff --git a/advice.c b/advice.c
> index 0018501b7b..e1264f525c 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -81,6 +81,7 @@ static struct {
> [ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure" },
> [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks" },
> [ADVICE_SPARSE_INDEX_EXPANDED] = { "sparseIndexExpanded" },
> + [ADVICE_STASH_BEFORE_CHECKOUT] = { "stashBeforeCheckout" },
> [ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning" },
> [ADVICE_STATUS_HINTS] = { "statusHints" },
> [ADVICE_STATUS_U_OPTION] = { "statusUoption" },
> @@ -312,3 +313,29 @@ void advise_on_moving_dirty_path(struct string_list *pathspec_list)
> "* Use \"git add --sparse <paths>\" to update the index\n"
> "* Use \"git sparse-checkout reapply\" to apply the sparsity rules"));
> }
> +
> +void advise_on_checkout_dirty_files(struct string_list *file_list)
Huh. So this patch wires up a new function and advice, but we don't ever
seem to use it. Am I missing something?
Patrick |
|
User |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): Phillip Wood <phillip.wood123@gmail.com> writes:
> If the intent is for the user to carry over the changes to the new
> branch then recommending "git checkout -m" might be more convenient
> rather than having to stash, checkout and unstash as three separate steps.
I personally would not recommend pushing "-m" to new people without
explaining its ramifications, though.
If "git stash pop" fails while a commit different from the original
is checked out, the working tree will get conflicts for you to
resolve, and that is the same as "git checkout -m". But the
conflict may turn out to be too complex that you might not be able
to cleanly resolve.
With a "git stash pop" that gets interrupted by a conflict, the
stash entry is not removed from the stash, so there is a clean
recourse to "git reset --hard" away the conflict and attempting to
unstash (either to the same commit or to a different base).
With "git checkout -m", on the other hand, there is no such
recourse. The conflicted working tree with the unmerged index is
all you get, and you get only a single chance to resolve it
correctly.
So...
|
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Thank you so much for looking into my PR and i believe advice.h is
used in the add.c file. And advice really helps young developers
understand what's wrong in their files because navigating git and
trying to find solutions is very difficult, causing them to go to ai
models making them copy pasting machines.
On Tue, 10 Mar 2026 at 18:46, Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Mar 10, 2026 at 10:59:24AM +0000, Arsh Srivastava via GitGitGadget wrote:
> > diff --git a/advice.c b/advice.c
> > index 0018501b7b..e1264f525c 100644
> > --- a/advice.c
> > +++ b/advice.c
> > @@ -81,6 +81,7 @@ static struct {
> > [ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure" },
> > [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks" },
> > [ADVICE_SPARSE_INDEX_EXPANDED] = { "sparseIndexExpanded" },
> > + [ADVICE_STASH_BEFORE_CHECKOUT] = { "stashBeforeCheckout" },
> > [ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning" },
> > [ADVICE_STATUS_HINTS] = { "statusHints" },
> > [ADVICE_STATUS_U_OPTION] = { "statusUoption" },
> > @@ -312,3 +313,29 @@ void advise_on_moving_dirty_path(struct string_list *pathspec_list)
> > "* Use \"git add --sparse <paths>\" to update the index\n"
> > "* Use \"git sparse-checkout reapply\" to apply the sparsity rules"));
> > }
> > +
> > +void advise_on_checkout_dirty_files(struct string_list *file_list)
>
> Huh. So this patch wires up a new function and advice, but we don't ever
> seem to use it. Am I missing something?
>
> Patrick |
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): As per the recommendation of Phillip Wood <phillip.wood123@gmail.com>
I have changed my files and added git checkout -m after understanding
its significance :)
On Tue, 10 Mar 2026 at 19:06, Junio C Hamano <gitster@pobox.com> wrote:
>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> > If the intent is for the user to carry over the changes to the new
> > branch then recommending "git checkout -m" might be more convenient
> > rather than having to stash, checkout and unstash as three separate steps.
>
> I personally would not recommend pushing "-m" to new people without
> explaining its ramifications, though.
>
> If "git stash pop" fails while a commit different from the original
> is checked out, the working tree will get conflicts for you to
> resolve, and that is the same as "git checkout -m". But the
> conflict may turn out to be too complex that you might not be able
> to cleanly resolve.
>
> With a "git stash pop" that gets interrupted by a conflict, the
> stash entry is not removed from the stash, so there is a clean
> recourse to "git reset --hard" away the conflict and attempting to
> unstash (either to the same commit or to a different base).
>
> With "git checkout -m", on the other hand, there is no such
> recourse. The conflicted working tree with the unmerged index is
> all you get, and you get only a single chance to resolve it
> correctly.
>
> So...
> |
| Shown when linkgit:git-rebase[1] skips a commit that has already | ||
| been cherry-picked onto the upstream branch. | ||
| sparseIndexExpanded:: | ||
| Shown when a sparse index is expanded to a full index, which is likely |
There was a problem hiding this comment.
Arsh Srivastava wrote on the Git mailing list (how to reply to this email):
Thank you so much for looking into my PR and i believe
> advice.h is used in the add.c file.
> And advice really helps young developers
understand what's wrong in their files because navigating git and
trying to find solutions is very difficult,
> causing them to go to ai models making them copy pasting machines.
On Tue, 10 Mar 2026 at 18:59, Arsh Srivastava via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Arsh Srivastava <arshsrivastava00@gmail.com>
>
> Add a new advice type ADVICE_STASH_BEFORE_CHECKOUT to guide users
> when they attempt to switch branches with local modifications that
> would be overwritten by the operation.
>
> This includes:
> > New ADVICE_STASH_BEFORE_CHECKOUT enum value in advice.h
> > Corresponding "stashBeforeCheckout" entry in advice_setting[]
> > New advise_on_checkout_dirty_files() function that lists the
> affected files and suggests using git stash push/pop
> > Documentation entry in Documentation/config/advice.txt
>
> The advice follows existing patterns established by
> advise_on_updating_sparse_paths() and can be silenced with:
>
> git config set advice.stashBeforeCheckout false
>
> Signed-off-by: Arsh Srivastava <arshsrivastava00@gmail.com>
> ---
> Documentation/config/advice.adoc | 5 +++++
> advice.c | 27 +++++++++++++++++++++++++++
> advice.h | 2 ++
> 3 files changed, 34 insertions(+)
>
> diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
> index 257db58918..8752e05636 100644
> --- a/Documentation/config/advice.adoc
> +++ b/Documentation/config/advice.adoc
> @@ -126,6 +126,11 @@ all advice messages.
> Shown when a sparse index is expanded to a full index, which is likely
> due to an unexpected set of files existing outside of the
> sparse-checkout.
> + stashBeforeCheckout::
> + Shown when the user attempts to switch branches but has
> + local modifications that would be overwritten by the
> + operation, to suggest using linkgit:git-stash[1] to
> + save changes before switching.
> statusAheadBehind::
> Shown when linkgit:git-status[1] computes the ahead/behind
> counts for a local ref compared to its remote tracking ref,
> diff --git a/advice.c b/advice.c
> index 0018501b7b..e1264f525c 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -81,6 +81,7 @@ static struct {
> [ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure" },
> [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks" },
> [ADVICE_SPARSE_INDEX_EXPANDED] = { "sparseIndexExpanded" },
> + [ADVICE_STASH_BEFORE_CHECKOUT] = { "stashBeforeCheckout" },
> [ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning" },
> [ADVICE_STATUS_HINTS] = { "statusHints" },
> [ADVICE_STATUS_U_OPTION] = { "statusUoption" },
> @@ -312,3 +313,29 @@ void advise_on_moving_dirty_path(struct string_list *pathspec_list)
> "* Use \"git add --sparse <paths>\" to update the index\n"
> "* Use \"git sparse-checkout reapply\" to apply the sparsity rules"));
> }
> +
> +void advise_on_checkout_dirty_files(struct string_list *file_list)
> +{
> + struct string_list_item *item;
> +
> + if (!file_list->nr)
> + return;
> +
> + fprintf(stderr, _("The following files have local modifications that would\n"
> + "be overwritten by switching branches:\n"));
> + for_each_string_list_item(item, file_list)
> + fprintf(stderr, "\t%s\n", item->string);
> +
> + advise_if_enabled(ADVICE_STASH_BEFORE_CHECKOUT,
> + _("You can save your local changes before switching by running:\n"
> + "\n"
> + "\tgit stash push\n"
> + "\n"
> + "Then restore them after switching with:\n"
> + "\n"
> + "\tgit stash pop\n"
> + "\n"
> + "Or to discard your local changes, use:\n"
> + "\n"
> + "\tgit checkout -- <file>"));
> +}
> diff --git a/advice.h b/advice.h
> index 8def280688..c035b5d8e3 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -48,6 +48,7 @@ enum advice_type {
> ADVICE_SET_UPSTREAM_FAILURE,
> ADVICE_SKIPPED_CHERRY_PICKS,
> ADVICE_SPARSE_INDEX_EXPANDED,
> + ADVICE_STASH_BEFORE_CHECKOUT,
> ADVICE_STATUS_AHEAD_BEHIND_WARNING,
> ADVICE_STATUS_HINTS,
> ADVICE_STATUS_U_OPTION,
> @@ -83,5 +84,6 @@ void NORETURN die_ff_impossible(void);
> void advise_on_updating_sparse_paths(struct string_list *pathspec_list);
> void detach_advice(const char *new_name);
> void advise_on_moving_dirty_path(struct string_list *pathspec_list);
> +void advise_on_checkout_dirty_files(struct string_list *file_list);
>
> #endif /* ADVICE_H */
> --
> gitgitgadget
>|
Patrick Steinhardt wrote on the Git mailing list (how to reply to this email): On Tue, Mar 10, 2026 at 07:06:39PM +0530, Arsh Srivastava wrote:
> On Tue, 10 Mar 2026 at 18:46, Patrick Steinhardt <ps@pks.im> wrote:
> > On Tue, Mar 10, 2026 at 10:59:24AM +0000, Arsh Srivastava via GitGitGadget wrote:
> > > diff --git a/advice.c b/advice.c
> > > index 0018501b7b..e1264f525c 100644
> > > --- a/advice.c
> > > +++ b/advice.c
> > > @@ -81,6 +81,7 @@ static struct {
> > > [ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure" },
> > > [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks" },
> > > [ADVICE_SPARSE_INDEX_EXPANDED] = { "sparseIndexExpanded" },
> > > + [ADVICE_STASH_BEFORE_CHECKOUT] = { "stashBeforeCheckout" },
> > > [ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning" },
> > > [ADVICE_STATUS_HINTS] = { "statusHints" },
> > > [ADVICE_STATUS_U_OPTION] = { "statusUoption" },
> > > @@ -312,3 +313,29 @@ void advise_on_moving_dirty_path(struct string_list *pathspec_list)
> > > "* Use \"git add --sparse <paths>\" to update the index\n"
> > > "* Use \"git sparse-checkout reapply\" to apply the sparsity rules"));
> > > }
> > > +
> > > +void advise_on_checkout_dirty_files(struct string_list *file_list)
> >
> > Huh. So this patch wires up a new function and advice, but we don't ever
> > seem to use it. Am I missing something?
>
> Thank you so much for looking into my PR and i believe advice.h is
> used in the add.c file. And advice really helps young developers
> understand what's wrong in their files because navigating git and
> trying to find solutions is very difficult, causing them to go to ai
> models making them copy pasting machines.
(Please note that we prefer bottom posting on this mailing list, where
your answer goes below the quoted context.)
It is used in "add.c", but not magically so. The function that you have
introduced is the only site that uses the new advice, but the function
is never called as far as I can see. So ultimately, the proposed change
does not have any effect on the user-observable behaviour.
Patrick |
|
Karthik Nayak wrote on the Git mailing list (how to reply to this email): "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Arsh Srivastava <arshsrivastava00@gmail.com>
>
> Add a new advice type ADVICE_STASH_BEFORE_CHECKOUT to guide users
> when they attempt to switch branches with local modifications that
> would be overwritten by the operation.
>
> This includes:
>> New ADVICE_STASH_BEFORE_CHECKOUT enum value in advice.h
>> Corresponding "stashBeforeCheckout" entry in advice_setting[]
>> New advise_on_checkout_dirty_files() function that lists the
> affected files and suggests using git stash push/pop
>> Documentation entry in Documentation/config/advice.txt
>
Nit: Did you mean to add bullet point here? '>' is generally used to
quote text. Perhaps use '-' or '*'.
[snip]
>
> Documentation/config/advice.adoc | 5 +++++
> advice.c | 27 +++++++++++++++++++++++++++
> advice.h | 2 ++
> 3 files changed, 34 insertions(+)
>
Hmm. Shouldn't there be changes which actually call the newly introduced
function? Also shouldn't there be tests added?
> diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
> index 257db58918..8752e05636 100644
> --- a/Documentation/config/advice.adoc
> +++ b/Documentation/config/advice.adoc
> @@ -126,6 +126,11 @@ all advice messages.
> Shown when a sparse index is expanded to a full index, which is likely
> due to an unexpected set of files existing outside of the
> sparse-checkout.
> + stashBeforeCheckout::
> + Shown when the user attempts to switch branches but has
> + local modifications that would be overwritten by the
> + operation, to suggest using linkgit:git-stash[1] to
> + save changes before switching.
Doesn't 'ADVICE_COMMIT_BEFORE_MERGE' already do this?
In one of my repos:
❯ git status
On branch master
Your branch is up to date with 'origin/master'.
nothing to commit, working tree clean
❯ echo "aldjf" >> LICENSE
❯ git status
On branch master
Your branch is up to date with 'origin/master'.
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: LICENSE
no changes added to commit (use "git add" and/or "git commit -a")
❯ git checkout 0-1-stable
error: Your local changes to the following files would be overwritten
by checkout:
LICENSE
Please commit your changes or stash them before you switch branches.
Aborting
So won't this simply be duplicating the same message?
> statusAheadBehind::
> Shown when linkgit:git-status[1] computes the ahead/behind
> counts for a local ref compared to its remote tracking ref,
> diff --git a/advice.c b/advice.c
> index 0018501b7b..e1264f525c 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -81,6 +81,7 @@ static struct {
> [ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure" },
> [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks" },
> [ADVICE_SPARSE_INDEX_EXPANDED] = { "sparseIndexExpanded" },
> + [ADVICE_STASH_BEFORE_CHECKOUT] = { "stashBeforeCheckout" },
> [ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning" },
> [ADVICE_STATUS_HINTS] = { "statusHints" },
> [ADVICE_STATUS_U_OPTION] = { "statusUoption" },
> @@ -312,3 +313,29 @@ void advise_on_moving_dirty_path(struct string_list *pathspec_list)
> "* Use \"git add --sparse <paths>\" to update the index\n"
> "* Use \"git sparse-checkout reapply\" to apply the sparsity rules"));
> }
> +
> +void advise_on_checkout_dirty_files(struct string_list *file_list)
> +{
> + struct string_list_item *item;
> +
> + if (!file_list->nr)
> + return;
> +
> + fprintf(stderr, _("The following files have local modifications that would\n"
> + "be overwritten by switching branches:\n"));
> + for_each_string_list_item(item, file_list)
> + fprintf(stderr, "\t%s\n", item->string);
> +
> + advise_if_enabled(ADVICE_STASH_BEFORE_CHECKOUT,
> + _("You can save your local changes before switching by running:\n"
> + "\n"
> + "\tgit stash push\n"
> + "\n"
> + "Then restore them after switching with:\n"
> + "\n"
> + "\tgit stash pop\n"
> + "\n"
> + "Or to discard your local changes, use:\n"
> + "\n"
> + "\tgit checkout -- <file>"));
> +}
This doesn't seem to be formatted with tabs. |
|
User |
ad6b111 to
1cc22f4
Compare
Author
|
/submit |
|
Submitted as pull.2233.v5.git.git.1773251369.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Junio C Hamano <gitster@pobox.com> writes :-
> provide perfect logic not drunken-man's-walk series.
> not a place for you to show how you made wrong turns before arriving at the final shape of the code.
> history for later developers to see in "git log" output to learn from
> what is "updation"
Thank you so much for your feedback.
Again terribly sorry I will rebase my commit so that it has perfect progression.
And updation is a noun first published in Oxford English Dictionary in 2018.
On Wed, 11 Mar 2026 at 22:08, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > This is my submission for microproject [GSOC]
> >
> > This patch adds a new advice type ADVICE_STASH_BEFORE_CHECKOUT to help users
> > when they attempt to switch branches with local modifications that would be
> > overwritten by the operation.
> >
> > The new advice follows the same patterns established by existing advice
> > functions such as advise_on_updating_sparse_paths(). When triggered, it
> > lists the affected files and suggests using git stash push/pop to save and
> > restore local changes.
> >
> > The advice can be silenced with:
> >
> > git config set advice.stashBeforeCheckout false
> >
> > Changes:
> >
> >> advice.h: add ADVICE_STASH_BEFORE_CHECKOUT enum value advice.c: add
> >> "stashBeforeCheckout" to advice_setting[] and implement
> >> advise_on_checkout_dirty_files() function
> >> Documentation/config/advice.adoc: document the new advice key
> >
> > Signed-off-by: Arsh Srivastava arshsrivastava00@gmail.com
>
> Even though no developer is perfect, when you are presenting your
> updated work, armed with wisdom borrowed from your reviewers'
> comments on your earlier attempts, you are expected to take the
> opportunity to pretend to have written a series of patches that are
> perfect logical progression towards the final shape of the code
> without detours, change of plans, and fixing earlier mistakes made
> in the series.
>
> Please do not throw a drunken-man's-walk series at us. For example,
> I see that [PATCH 3/5] literally removes what was added by earlier
> patches. This is not a place for you to show how you made wrong
> turns before arriving at the final shape of the code.
>
> The final series accepted by the project will have to stay in our
> history for later developers to see in "git log" output to learn
> from, and a series being clean logical progression is a must for
> that to happen.
>
> Also, what is "updation"? Is it a standard English word, or some
> dialect of an LLM origin?
>
> > Arsh Srivastava (5):
> > advice: add stashBeforeCheckout advice for dirty branch switches
> > advice: add stashBeforeCheckout advice for dirty branch switches
> > [GSOC]
> > unpack-trees: suggesting 'git checkout -m <branch>' with its
> > repercussions
> > Updating tests and unpack-tress.c [GSOC]
> > File updation [GSOC]
> >
> > t/t6439-merge-co-error-msgs.sh | 6 ++++++
> > t/t7406-submodule-update.sh | 3 +++
> > unpack-trees.c | 9 +++++++--
> > 3 files changed, 16 insertions(+), 2 deletions(-)
> >
> >
> > base-commit: d181b9354cf85b44455ce3ca9e6af0b9559e0ae2
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2233%2FArsh123344423%2Fadvice_on_checkout_dirty_files-v4
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2233/Arsh123344423/advice_on_checkout_dirty_files-v4
> > Pull-Request: https://github.com/git/git/pull/2233
> >
> > Range-diff vs v3:
> >
> > 1: eb5639dbc3 = 1: eb5639dbc3 advice: add stashBeforeCheckout advice for dirty branch switches
> > 2: e88c851701 = 2: e88c851701 advice: add stashBeforeCheckout advice for dirty branch switches [GSOC]
> > -: ---------- > 3: 4237b9667d unpack-trees: suggesting 'git checkout -m <branch>' with its repercussions
> > -: ---------- > 4: b25ea22410 Updating tests and unpack-tress.c [GSOC]
> > -: ---------- > 5: 2ef7d5a3d6 File updation [GSOC] |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Changes:
>
>> advice.h: add ADVICE_STASH_BEFORE_CHECKOUT enum value advice.c: add
>> "stashBeforeCheckout" to advice_setting[] and implement
>> advise_on_checkout_dirty_files() function
>> Documentation/config/advice.adoc: document the new advice key
>
> Signed-off-by: Arsh Srivastava arshsrivastava00@gmail.com
>
> Arsh Srivastava (3):
> advice: add stashBeforeCheckout advice for dirty branch switches
> advice: add stashBeforeCheckout advice for dirty branch switches
> [GSOC]
> unpack-trees: suggesting 'git checkout -m <branch>' with its
> repercussions
I still see that [PATCH 3/3] literally removes what was added by
earlier patches. ADVICE_STASH_BEFORE_CHECKOUT is added to
advice.[ch] and stashBeforeCheckOut is added to
Documentation/config/adivce.adoc in [1/3], and then they are removed
in [3/3]. If your final solution does not involve such an advice,
then do not even add it in an earlier patch, only to retract it and
replace it with something else in a later patch. If the reason why
that "something else" replaces the advice is because it is a better
solution to the problem you initially started to solve than the
advice message added in [1/3], then just go straight to that
"something else", without adding and removing the advice mechanism.
That is what "perfect logical progression without detours, change of
plans, and fixes earlier mistake" is about.
In other words, please do not throw a drunken-man's-walk series at
us. Pretend to be a perfect developer.
The final series accepted by the project will have to stay in our
history for later developers to see in "git log" output to learn
from, and a series being clean logical progression is a must for
that to happen.
|
1cc22f4 to
e0ba5fa
Compare
Author
|
/submit |
|
Submitted as pull.2233.v6.git.git.1773288013936.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Junio C Hamano <gitster@pobox.com> writes :
> He still see 3 patches for a work that can be done by one.
> That is what "perfect logical progression without detours, change of
> plans, and fixes earlier mistake" is about.
> please do not throw a drunken-man's-walk series at
> us. Pretend to be a perfect developer
I understand now sorry for the inconvenience.
Your mail also specified the same thing but I was not able to truly
grasp the essence of that mail.
Thank you for explaining it again to me, I now have clear
understanding of what is expected from me.
I will create one single commit :) .
I also saw that git via git gadget forwards all my commits making it
messy and not ideal.
I will make a clean single commit that will look perfect and wouldn't
make such a messy commit history.
On Thu, 12 Mar 2026 at 06:32, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > Changes:
> >
> >> advice.h: add ADVICE_STASH_BEFORE_CHECKOUT enum value advice.c: add
> >> "stashBeforeCheckout" to advice_setting[] and implement
> >> advise_on_checkout_dirty_files() function
> >> Documentation/config/advice.adoc: document the new advice key
> >
> > Signed-off-by: Arsh Srivastava arshsrivastava00@gmail.com
> >
> > Arsh Srivastava (3):
> > advice: add stashBeforeCheckout advice for dirty branch switches
> > advice: add stashBeforeCheckout advice for dirty branch switches
> > [GSOC]
> > unpack-trees: suggesting 'git checkout -m <branch>' with its
> > repercussions
>
> I still see that [PATCH 3/3] literally removes what was added by
> earlier patches. ADVICE_STASH_BEFORE_CHECKOUT is added to
> advice.[ch] and stashBeforeCheckOut is added to
> Documentation/config/adivce.adoc in [1/3], and then they are removed
> in [3/3]. If your final solution does not involve such an advice,
> then do not even add it in an earlier patch, only to retract it and
> replace it with something else in a later patch. If the reason why
> that "something else" replaces the advice is because it is a better
> solution to the problem you initially started to solve than the
> advice message added in [1/3], then just go straight to that
> "something else", without adding and removing the advice mechanism.
>
> That is what "perfect logical progression without detours, change of
> plans, and fixes earlier mistake" is about.
>
> In other words, please do not throw a drunken-man's-walk series at
> us. Pretend to be a perfect developer.
>
> The final series accepted by the project will have to stay in our
> history for later developers to see in "git log" output to learn
> from, and a series being clean logical progression is a must for
> that to happen.
> |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Arsh Srivastava <arshsrivastava00@gmail.com>
>
> This comment is an extention to the already existing stash comment.
> Added updated comment over the already existing function
> "setup_unpack_trees_porcelain" with "git checkout -m"
> and its repercussions
> I have also mentioned the repercussions of using "-m".
>
> Signed-off-by: Arsh Srivastava arshsrivastava00@gmail.com
The commit message should focus on the "why" and "what" from a user
perspective, following the project's standard format (problem
description, then solution).
Consider a more standard phrasing:
unpack-trees: suggest 'git checkout -m' when checkout fails
When a branch switch fails due to local changes, we suggest
stashing or committing. However, 'git checkout -m' is a valid
alternative for users who wish to carry their changes over via a
merge.
Update the advice message to suggest this option, while
including a warning about the potential for data loss if a hard
reset is performed after a conflicted merge.
Also, note that "extention" is a typo; it should be "extension".
Having said that, I am not sure if we want to suggest "checkout -m"
in this situation after all.
The added message is quite long:
> Try using 'git checkout -m <branch>' for a quick fix.
> Please Note :- that using -m (merge) will not save your changes,
> rather would directly merge them.
> Meaning if you are not able to resolve conflicts and does --hard
> reset your local changes would be gone.
When advice requires a multi-line warning about potential data loss,
it's often a sign that the operation being suggested isn't suitable
for general advice. The goal of these messages should be to provide
a clear, safe next step, not a list of advanced alternatives with
caveats. After all, the users who need such an "it failed, now what
should I do to recover?" message the most are relatively
inexperienced users and we do not want the advice to be
overwhelming.
The primary concern is that 'git checkout -m' is a high-stakes
operation compared to 'git stash'.
- When a user uses 'git stash', their changes are recorded in a
stash entry. If 'git stash pop' later results in conflicts they
cannot resolve, the user can always 'git reset --hard' to get back
to a clean state, knowing their original changes are still safe in
the stash entry, which they can re-attempt to use later.
- In contrast, 'git checkout -m' performs the merge directly in
the working tree. If conflicts arise, the original local changes
are immediately replaced by conflict markers. Unlike stash, there
is no "undo" record. If the user realizes they are in over their
head, they cannot simply "abort" to get their original changes
back. They have only one chance to resolve it correctly, and they
have to do so right there.
Suggesting this "one-shot" approach to a user who is already in a
state of friction (and likely less experienced) might be providing
them with a "foot-gun" rather than a helpful tip. Generally, advice
that nudges users toward the safest "golden paths" like stashing or
committing is preferred.
For a microproject, you've successfully demonstrated that you can
modify the advice system and update the test suite. However, for the
health of the project's usability, it might be better to drop the
'checkout -m' suggestion and instead focus on making the existing
'stash' and 'commit' advice as clear and helpful as possible.
Thanks.
|
db3f9ac to
f34c202
Compare
6B28
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Junio C Hamano <gitster@pobox.com> writes :
> The commit message should focus on the "why" and "what" from a user perspective, following the project's standard format (problem description, then solution).
> Also showed an example for the same.
> Also, note that "extention" is a typo; it should be "extension".
> Having said that, I am not sure if we want to suggest "checkout -m" in this situation after all.
> Pointed out the difference between "stash" and "checkout -m".
> When advice requires a multi-line warning about potential data loss.
> The goal of these messages should be to provide a clear, safe next step, not a list of advanced alternatives with caveats.
> After all, the users who need such an "it failed, now what should I do to recover?" message the most are relatively inexperienced users and we do not want the advice to be overwhelming.
> Suggesting this "one-shot" approach to a user who is already in a state of friction (and likely less experienced) might be providing them with a "foot-gun" rather than a helpful tip. Generally, advice that nudges users toward the safest "golden paths" like stashing or committing is preferred.
> For a microproject, you've successfully demonstrated that you can modify the advice system and update the test suite.
> it might be better to drop the 'checkout -m' suggestion and instead focus on making the existing 'stash' and 'commit' advice as clear and helpful as possible.
Thank you so much for the valuable feedback.
I will redefine my advice and will make it more precise and will
change it to git stash which is truly more beneficial for the new
users.
Also will update my commit so that it is properly structured with
format first "why" then "what".
I will create a v7 with all these changes.
I am really obliged.
Thanks again for the guidance.
On Thu, 12 Mar 2026 at 21:36, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Arsh Srivastava <arshsrivastava00@gmail.com>
> >
> > This comment is an extention to the already existing stash comment.
> > Added updated comment over the already existing function
> > "setup_unpack_trees_porcelain" with "git checkout -m"
> > and its repercussions
> > I have also mentioned the repercussions of using "-m".
> >
> > Signed-off-by: Arsh Srivastava arshsrivastava00@gmail.com
>
> The commit message should focus on the "why" and "what" from a user
> perspective, following the project's standard format (problem
> description, then solution).
>
> Consider a more standard phrasing:
>
> unpack-trees: suggest 'git checkout -m' when checkout fails
>
> When a branch switch fails due to local changes, we suggest
> stashing or committing. However, 'git checkout -m' is a valid
> alternative for users who wish to carry their changes over via a
> merge.
>
> Update the advice message to suggest this option, while
> including a warning about the potential for data loss if a hard
> reset is performed after a conflicted merge.
>
> Also, note that "extention" is a typo; it should be "extension".
>
> Having said that, I am not sure if we want to suggest "checkout -m"
> in this situation after all.
>
> The added message is quite long:
>
> > Try using 'git checkout -m <branch>' for a quick fix.
> > Please Note :- that using -m (merge) will not save your changes,
> > rather would directly merge them.
> > Meaning if you are not able to resolve conflicts and does --hard
> > reset your local changes would be gone.
>
> When advice requires a multi-line warning about potential data loss,
> it's often a sign that the operation being suggested isn't suitable
> for general advice. The goal of these messages should be to provide
> a clear, safe next step, not a list of advanced alternatives with
> caveats. After all, the users who need such an "it failed, now what
> should I do to recover?" message the most are relatively
> inexperienced users and we do not want the advice to be
> overwhelming.
>
> The primary concern is that 'git checkout -m' is a high-stakes
> operation compared to 'git stash'.
>
> - When a user uses 'git stash', their changes are recorded in a
> stash entry. If 'git stash pop' later results in conflicts they
> cannot resolve, the user can always 'git reset --hard' to get back
> to a clean state, knowing their original changes are still safe in
> the stash entry, which they can re-attempt to use later.
>
> - In contrast, 'git checkout -m' performs the merge directly in
> the working tree. If conflicts arise, the original local changes
> are immediately replaced by conflict markers. Unlike stash, there
> is no "undo" record. If the user realizes they are in over their
> head, they cannot simply "abort" to get their original changes
> back. They have only one chance to resolve it correctly, and they
> have to do so right there.
>
> Suggesting this "one-shot" approach to a user who is already in a
> state of friction (and likely less experienced) might be providing
> them with a "foot-gun" rather than a helpful tip. Generally, advice
> that nudges users toward the safest "golden paths" like stashing or
> committing is preferred.
>
> For a microproject, you've successfully demonstrated that you can
> modify the advice system and update the test suite. However, for the
> health of the project's usability, it might be better to drop the
> 'checkout -m' suggestion and instead focus on making the existing
> 'stash' and 'commit' advice as clear and helpful as possible.
>
> Thanks.
> |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): Arsh Srivastava <arshsrivastava00@gmail.com> writes:
> Junio C Hamano <gitster@pobox.com> writes :
>> The commit message should focus on the "why" and "what" from a user perspective, following the project's standard format (problem description, then solution).
>> Also showed an example for the same.
>> Also, note that "extention" is a typo; it should be "extension".
>> Having said that, I am not sure if we want to suggest "checkout -m" in this situation after all.
>> Pointed out the difference between "stash" and "checkout -m".
>> When advice requires a multi-line warning about potential data loss.
>> The goal of these messages should be to provide a clear, safe next step, not a list of advanced alternatives with caveats.
>> After all, the users who need such an "it failed, now what should I do to recover?" message the most are relatively inexperienced users and we do not want the advice to be overwhelming.
>> Suggesting this "one-shot" approach to a user who is already in a state of friction (and likely less experienced) might be providing them with a "foot-gun" rather than a helpful tip. Generally, advice that nudges users toward the safest "golden paths" like stashing or committing is preferred.
>> For a microproject, you've successfully demonstrated that you can modify the advice system and update the test suite.
>> it might be better to drop the 'checkout -m' suggestion and instead focus on making the existing 'stash' and 'commit' advice as clear and helpful as possible.
I wonder where this came from, as it is quite unusual to have a
rephrased summary of what you respond to. Is this LLM-generated
summary that was copied-and-pasted without much human brain effort?
What is more usual is to quote the message you are responding to,
trim the parts you are not going to comment on and not necessary for
bystanders to read in order to understand your response, and then
sprinkle your comments in between the parts of the quoted message,
instead of saying only your thing before the message you are
responding to without trimming (which is highly frowned upon, and
people often tell you not to "top-post").
|
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Junio C Hamano <gitster@pobox.com> write:
> I wonder where this came from, as it is quite unusual to have a rephrased summary of what you respond to.
> Is this LLM-generated summary that was copied-and-pasted without much human brain effort?
> What is more usual is to quote the message you are responding to.
Actually as suspicious as it looks the mail was written by me only and
I thought that each line was as important as other and
I wanted to reply to each point you mentioned
Sorry for shortening your response
next time I will make sure that I will make quotations direct.
On Fri, 13 Mar 2026 at 00:26, Junio C Hamano <gitster@pobox.com> wrote:
>
> Arsh Srivastava <arshsrivastava00@gmail.com> writes:
>
> > Junio C Hamano <gitster@pobox.com> writes :
> >> The commit message should focus on the "why" and "what" from a user perspective, following the project's standard format (problem description, then solution).
> >> Also showed an example for the same.
> >> Also, note that "extention" is a typo; it should be "extension".
> >> Having said that, I am not sure if we want to suggest "checkout -m" in this situation after all.
> >> Pointed out the difference between "stash" and "checkout -m".
> >> When advice requires a multi-line warning about potential data loss.
> >> The goal of these messages should be to provide a clear, safe next step, not a list of advanced alternatives with caveats.
> >> After all, the users who need such an "it failed, now what should I do to recover?" message the most are relatively inexperienced users and we do not want the advice to be overwhelming.
> >> Suggesting this "one-shot" approach to a user who is already in a state of friction (and likely less experienced) might be providing them with a "foot-gun" rather than a helpful tip. Generally, advice that nudges users toward the safest "golden paths" like stashing or committing is preferred.
> >> For a microproject, you've successfully demonstrated that you can modify the advice system and update the test suite.
> >> it might be better to drop the 'checkout -m' suggestion and instead focus on making the existing 'stash' and 'commit' advice as clear and helpful as possible.
>
> I wonder where this came from, as it is quite unusual to have a
> rephrased summary of what you respond to. Is this LLM-generated
> summary that was copied-and-pasted without much human brain effort?
>
> What is more usual is to quote the message you are responding to,
> trim the parts you are not going to comment on and not necessary for
> bystanders to read in order to understand your response, and then
> sprinkle your comments in between the parts of the quoted message,
> instead of saying only your thing before the message you are
> responding to without trimming (which is highly frowned upon, and
> people often tell you not to "top-post").
> |
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. 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. Signed-off-by: Arsh Srivastava <arshsrivastava00@gmail.com>
f34c202 to
72cb550
Compare
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): Arsh Srivastava <arshsrivastava00@gmail.com> writes:
> Junio C Hamano <gitster@pobox.com> write:
>> I wonder where this came from, as it is quite unusual to have a rephrased summary of what you respond to.
>> Is this LLM-generated summary that was copied-and-pasted without much human brain effort?
>> What is more usual is to quote the message you are responding to.
>
> Actually as suspicious as it looks the mail was written by me only and
> I thought that each line was as important as other and
> I wanted to reply to each point you mentioned
> Sorry for shortening your response
> next time I will make sure that I will make quotations direct.
That wasn't what I meant, and you are still top-posting X-<. |
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Junio C Hamano <gitster@pobox.com> write:
> That wasn't what I meant, and you are still top-posting X-<.
Oh.. I understand now I mistakenly again used top-posting mail (-<
which is frustrating
I will use the recommended method only from now on. : )
On Fri, 13 Mar 2026 at 00:37, Junio C Hamano <gitster@pobox.com> wrote:
>
> Arsh Srivastava <arshsrivastava00@gmail.com> writes:
>
> > Junio C Hamano <gitster@pobox.com> write:
> >> I wonder where this came from, as it is quite unusual to have a rephrased summary of what you respond to.
> >> Is this LLM-generated summary that was copied-and-pasted without much human brain effort?
> >> What is more usual is to quote the message you are responding to.
> >
> > Actually as suspicious as it looks the mail was written by me only and
> > I thought that each line was as important as other and
> > I wanted to reply to each point you mentioned
> > Sorry for shortening your response
> > next time I will make sure that I will make quotations direct.
>
> That wasn't what I meant, and you are still top-posting X-<. |
Author
|
/submit |
|
Submitted as pull.2233.v7.git.git.1773345901659.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> 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 <branch>' 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 <branch>' 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 <branch> && 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.
|
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Junio C Hamano <gitster@pobox.com> 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 <gitster@pobox.com> wrote:
>
> "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> 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 <branch>' 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 <branch>' 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 <branch> && 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.
> |
|
Karthik Nayak wrote on the Git mailing list (how to reply to this email): Arsh Srivastava <arshsrivastava00@gmail.com> writes:
> Junio C Hamano <gitster@pobox.com> 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.
>
From the micro-project information [1] for GSoC we have:
The coding part of the microproject should be very small (say, 10-30
minutes). We don’t require that your patch be accepted into the
“master” branch by the time of your formal application; we mostly want
to see that you have a basic level of competence and especially the
ability to interact with the other Git developers.
As such and seeing Junio's previous response, I would say that no
further work is required here.
[1]: https://git.github.io/General-Microproject-Information/
[snip] |
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Arsh Srivastava <arshsrivastava00@gmail.com> writes:
> 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.
Actually the above text should be :-
I understand, git wants people to explore the available
change options and help them make logical decisions rather
than pushing them with some unneeded commands.
On Fri, 13 Mar 2026 at 08:43, Arsh Srivastava
<arshsrivastava00@gmail.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> 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 <gitster@pobox.com> wrote:
> >
> > "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> 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 <branch>' 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 <branch>' 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 <branch> && 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.
> > |
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Karthik Nayak <karthik.188@gmail.com> writes :
> We don’t require that your patch be accepted into the
> “master” branch by the time of your formal application
Understandable
> we mostly want to see that you have a basic level of
> competence and especially the
> ability to interact with the other Git developers.
Truly this is something that is very important in any
organization.
> As such and seeing Junio's previous response, I would say that no
> further work is required here.
Thank you so much for your valuable guidance, I will now
draft a formal proposal for GSoC and I look forward to working
with you in the future too. :)
On Fri, 13 Mar 2026 at 16:13, Karthik Nayak <karthik.188@gmail.com> wrote:
>
> Arsh Srivastava <arshsrivastava00@gmail.com> writes:
>
> > Junio C Hamano <gitster@pobox.com> 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
|
|
Arsh Srivastava wrote on the Git mailing list (how to reply to this email): Arsh Srivastava <arshsrivastava00@gmail.com> writes:
> I understand, git wants people to not explore the available
This was a typo the real text is :
I understand, git wants people to explore the available
On Fri, 13 Mar 2026 at 08:43, Arsh Srivastava
<arshsrivastava00@gmail.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> 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 <gitster@pobox.com> wrote:
> >
> > "Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> 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 <branch>' 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 <branch>' 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 <branch> && 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.
> > |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): Karthik Nayak <karthik.188@gmail.com> writes:
> From the micro-project information [1] for GSoC we have:
>
> The coding part of the microproject should be very small (say, 10-30
> minutes). We don’t require that your patch be accepted into the
> “master” branch by the time of your formal application; we mostly want
> to see that you have a basic level of competence and especially the
> ability to interact with the other Git developers.
>
> As such and seeing Junio's previous response, I would say that no
> further work is required here.
>
> [1]: https://git.github.io/General-Microproject-Information/
Thanks ;-). |
|
Konstantin Ryabitsev wrote on the Git mailing list (how to reply to this email): On Tue, Mar 10, 2026 at 10:09:25AM -0700, Karthik Nayak wrote:
> >> I will rework the patch in that direction and send a v4.
> >>
> >> Signed-off-by: Arsh Srivastava <arshsrivastava00@gmail.com>
> >
> > Just a comment by a bystander, but it confuses me quite a lot to see
> > in-body "Subject:" and "Sign-off" in a message that is *not* a patch
> > at all. What are you signing off with this signature?
>
> Tangentially, I know that b4 adds a "Sign-off" to the cover message.
This is a feature, because some subsystems use the cover message as the source
for the merge commit, so the cover message *is* a commit despite not being a
patch. :)
Regards,
--
KR |
|
User |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is my submission for microproject [GSOC]
This patch extends the current message to help users better understand risks and alternatives to stashing their changes.
The alternative consists of
This extended message is tied to the existing advice.commitBeforeMerge configuration and can be silenced with:
Changes:
Signed-off-by: Arsh Srivastava arshsrivastava00@gmail.com
cc: Phillip Wood phillip.wood123@gmail.com
cc: Arsh Srivastava arshsrivastava00@gmail.com
cc: Patrick Steinhardt ps@pks.im
cc: Karthik Nayak karthik.188@gmail.com
cc: Konstantin Ryabitsev mricon@kernel.org