Skip to content

checkout: don't revert file on ambiguous tracking branches #504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

SyntevoAlex
Copy link

@SyntevoAlex SyntevoAlex commented Dec 30, 2019

This is an improved version of [1]; I tried to clarify the commit message.
CC'ing authors of previous commits mentioned in my commit message.

[1] https://public-inbox.org/git/[email protected]/T/#u

Cc: Nguyễn Thái Ngọc Duy <[email protected]>
Cc: Ævar Arnfjörð Bjarmason <[email protected]>

This is done for the next commit to avoid crazy 7x tab code padding.

Signed-off-by: Alexandr Miloslavskiy <[email protected]>
For easier understanding, here are the existing good scenarios:

  1) Have *no* file 'foo', *no* local branch 'foo' and a *single*
     remote branch 'foo'
  2) `git checkout foo` will create local branch foo, see [1]

  and

  1) Have *a* file 'foo', *no* local branch 'foo' and a *single*
     remote branch 'foo'
  2) `git checkout foo` will complain, see [3]

This patch prevents the following scenario:

  1) Have *a* file 'foo', *no* local branch 'foo' and *multiple*
     remote branches 'foo'
  2) `git checkout foo` will successfully... revert contents of
     file `foo`!

That is, adding another remote suddenly changes behavior significantly,
which is a surprise at best and could go unnoticed by user at worst.
Please see [3] which gives some real world complaints.

To my understanding, fix in [3] overlooked the case of multiple remotes,
and the whole behavior of falling back to reverting file was never
intended:

  [1] introduces the unexpected behavior. Before, there was fallback
  from not-a-ref to pathspec. This is reasonable fallback. After, there
  is another fallback from ambiguous-remote to pathspec. I understand
  that it was a copy&paste oversight.

  [2] noticed the unexpected behavior but chose to semi-document it
  instead of forbidding, because the goal of the patch series was
  focused on something else.

  [3] adds `die()` when there is ambiguity between branch and file. The
  case of multiple tracking branches is seemingly overlooked.

The new behavior: if there is no local branch and multiple remote
candidates, just die() and don't try reverting file whether it
exists (prevents surprise) or not (improves error message).

[1] Commit 70c9ac2 ("DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"" 2009-10-18)
    https://public-inbox.org/git/[email protected]/
[2] Commit ad8d510 ("checkout: add advice for ambiguous "checkout <branch>"", 2018-06-05)
    https://public-inbox.org/git/[email protected]/
[3] Commit be4908f ("checkout: disambiguate dwim tracking branches and local files", 2018-11-13)
    https://public-inbox.org/git/[email protected]/

Signed-off-by: Alexandr Miloslavskiy <[email protected]>
@SyntevoAlex
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 30, 2019

Submitted as [email protected]

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 7, 2020

This branch is now known as am/checkout-file-and-ref-ref-ambiguity.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 7, 2020

This patch series was integrated into pu via git@1ed3ace.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 8, 2020

This patch series was integrated into pu via git@3aca945.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 9, 2020

This patch series was integrated into pu via git@15da7d4.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 9, 2020

This patch series was integrated into pu via git@ea598e5.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 10, 2020

This patch series was integrated into pu via git@8b7ad33.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 10, 2020

This patch series was integrated into pu via git@dda7344.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 13, 2020

This patch series was integrated into pu via git@386ba44.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 13, 2020

This patch series was integrated into pu via git@fa531ec.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 13, 2020

This patch series was integrated into pu via git@b0a137d.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 15, 2020

This patch series was integrated into pu via git@a8e7cd6.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 17, 2020

This patch series was integrated into pu via git@4958d71.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 17, 2020

This patch series was integrated into pu via git@25f1c9b.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 22, 2020

This patch series was integrated into pu via git@f3c06eb.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 24, 2020

This patch series was integrated into pu via git@ea59758.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 27, 2020

This patch series was integrated into pu via git@38c67b9.

@gitgitgadget
Copy link

gitgitgadget bot commented Jan 27, 2020

This patch series was integrated into next via git@daf1dad.

@gitgitgadget gitgitgadget bot added the next label Jan 27, 2020
@gitgitgadget
Copy link

gitgitgadget bot commented Jan 30, 2020

This patch series was integrated into pu via git@6b09d00.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 5, 2020

This patch series was integrated into pu via git@d0e70cd.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 5, 2020

This patch series was integrated into next via git@d0e70cd.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 5, 2020

This patch series was integrated into master via git@d0e70cd.

@gitgitgadget gitgitgadget bot added the master label Feb 5, 2020
@gitgitgadget gitgitgadget bot closed this Feb 5, 2020
@gitgitgadget
Copy link

gitgitgadget bot commented Feb 5, 2020

Closed via d0e70cd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant