On 27.11.2019 16:43, Ævar Arnfjörð Bjarmason wrote:
I'll reserve judgement on whether we really should do this for now, my
current opinion on the matter is undefined as I haven't re-paged this
behavior of checkout into my brain.
But a giant red flag here for me is that you say "I understand that this
was never intended".
Just from a cursory look at this that's not true, for better or worse it
*is* intended behavior. Most of the code you're moving around here is
what I added in ad8d5104b4 ("checkout: add advice for ambiguous
"checkout <branch>"", 2018-06-05), and the very start of that commit
message refers to the checkout documentation we have that explicitly
documents this edge case.
Digging a bit further reveals that we've had this behavior (again,
intended, not emergent) since 70c9ac2f19 ("DWIM "git checkout frotz" to
"git checkout -b frotz origin/frotz"", 2009-10-18), and had it
documented since 00bb4378c7 ("Documentation/git-checkout.txt: document
70c9ac2 behavior", 2012-12-17).
So at the very least I'd say you need a v2 where you amend the relevant
docs & commit message to make a case to the effect of "we've had this
since 2009, but it was never really all that important etc.".
I'm sorry, can you please clarify?
My patch addresses the situation where there are *multiple* remote
candidates *and* a file with the same name.
My feeling is that in this case, reverting a file is an unintended surprise.
I understand previous patches this way:
ad8d5104 - patch series is mostly for "if there are *multiple* remotes,
disambiguate via checkout.defaultRemote". This essentially
converts the case of multiple remotes into a single remote.
However, this also semi-documents what I'm now preventing.
Was that really intended?
70c9ac2f - if there is *one* remote, DWIM it.
This isn't what I'm changing.
be4908f1 - if there is *one* remote *and* file, die().
This is what I'm extending further.
If the prevented behavior is documented, could you please quote it
explicitly?
Such a change should also be changing the docs etc. added in 8d7b558bae
("checkout & worktree: introduce checkout.defaultRemote",
2018-06-05). With this series our docs don't make a lot of sense anymore
& don't describe the behavior with the patches applied.
To my understanding, my patch doesn't affect those pieces of docs?
Please correct me if I'm wrong.
