On 18.12.2019 20:18, Junio C Hamano wrote:
`has_dash_dash` unexpectedly takes `opts->accept_pathspec` into account.
You also touched the code that depends on opts->accept_pathspec in
the earlier step 1/5; these two steps seem harder to reason about
than necessary---I wonder if it is easier to explain and understand
if these two steps are merged into one and explained together?
Yes, that sounds like a good idea! In V3 in other topic [1] I have
shuffled the changes between commits for easier understanding.
+ if (has_dash_dash)
+ expect_commit_only = 1;
Non-standard indentation here.
Sorry!
+ opts->count_checkout_paths = !opts->quiet && !expect_commit_only;
OK. count_checkout_paths is relevant only when checking out paths
out of a tree-ish, so "expect-commit-only" would be false in such a
situation. On the other hand, if we were checking out a branch (or
detaching), we must have a commit and a tree-ish is insufficient,
so expect_commit_only would be true in such a case.
Makes sense. I am wondering if we still need has_dash_dash, and
also if expect_commit_only is the best name for the variable.
It seems that indeed, the variable could use an even better name. It's
<commit> as opposed to <pathspec>, and not as opposed to <tree-ish>. I
have renamed the variable again in V3 in other topic [1].
----
[1]
https://lore.kernel.org/git/pull.490.v3.git.1576778515.gitgitgadget@xxxxxxxxx/
