* Re: git switch --force vs --discard-changes: docs don't match behavior
2025-03-17 17:19 ` Junio C Hamano
@ 2025-03-17 17:39 ` Junio C Hamano
2025-03-17 18:13 ` David Mandelberg
1 sibling, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2025-03-17 17:39 UTC (permalink / raw)
To: David Mandelberg; +Cc: git, Elijah Newren
Junio C Hamano <gitster@pobox.com> writes:
> I do not have a strong opinion either way. It may appear to some
> users that giving a finer grained control is a merit. Even when you
> are willing to throw away changes to already tracked content,
> getting stopped when you may lose a totally untracked thing might be
> nicer.
>
> On the other hand, I suspect to many others this finer grained
> control does not give much value while adding more confusion.
>
> I am obviously biased because I am accustomed not to have this
> distinction and accept "checkout -f" as a reasonable way to force
> switching to another branch discarding any and all local
> modifications including untracked new files that get in the way,
> though. But I do not feel strongly enough to say that the behaviour
> and the feature itself is misguided and we should rip it out. As
> long as that "finer grained control" is working in a consistent and
> explainable way, I'd actually vote for fixing the documentation to
> explain how "--discard-changes" is a bit milder than "--force'.
It seems that this was designed from day one to work this way, or
this was buggy from day one to work this way. Take your pick ;-)
When 3ec37ad1 (switch: add --discard-changes, 2019-03-29) introduced
the --discard-changes option, it seems that it was intended to be
truly the synonym, but the code change was done in a somewhat
confusing way and added an extra bit (a new .discard_changes member
was added, and is initialized by the value given to the .force
member. Many existing conditionals that looked at the .force bit
was changed to look at the .discard_changes bit, but not all of
them.
It did not offer the protection against overwriting untracked files
in builtin/checkout.c:reset_tree(). The .reset member was always
set to 1 to allow it.
Later 480d3d6b (Change unpack_trees' 'reset' flag into an enum,
2021-09-27) added the "protect against overwriting untracked files"
feature, and made it pay attention to the "force" option, not the
other member. That is where this discrepancy was introduced. In
retrospect, it might have been better if 3ec37ad1 removed .force and
kept only "--force" at the UI layer, internally used the new
.discard_changes everywhere. Or it would have been even better if
the commit did not happen at all, and added "--discard-changes" only
at the UI layer. Then the discrepancy would never have happened.
The author of 480d3d6b Cc'ed to ask for the original intention for
reference, but that is merely to satisfy my curiosity. At this
point, I think it is way too late for the original intention to sway
the conclusion, as existing users of "switch" may be relying on the
current behaviour. The best course of action may be to update the
documentation to match the behaviour.
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: git switch --force vs --discard-changes: docs don't match behavior
2025-03-17 17:19 ` Junio C Hamano
2025-03-17 17:39 ` Junio C Hamano
@ 2025-03-17 18:13 ` David Mandelberg
1 sibling, 0 replies; 4+ messages in thread
From: David Mandelberg @ 2025-03-17 18:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Op 2025-03-17 om 13:19 schreef Junio C Hamano:
> David Mandelberg <david@mandelberg.org> writes:
>
>> $ touch Makefile
>
> You'd need to explain this example a bit better. The reproduction
> must be done in a repository where the master branch has Makefile
> tracked, you are not on the master branch, and the commit you have
> checked out does not have Makefile tracked. IOW, this is not making
> Makefile stat-dirty, but is creating a new untracked file.
Sorry about that. I had originally meant to copy/paste all the commands
I ran, but the `git fetch` command produced a lot of output and I
decided to only paste the ones that showed the bug, not the setup ones.
Here are the commands I had run, starting in an empty directory:
git init
git remote add origin https://github.com/git/git.git
git fetch origin
# a few status and show commands, but those hopefully aren't important
So I actually didn't have any commit checked out, but I think having a
commit checked out that did not have a Makefile would have done the same
thing.
>> Is this a bug in the code or documentation?
>
> I do not have a strong opinion either way. It may appear to some
> users that giving a finer grained control is a merit. Even when you
> are willing to throw away changes to already tracked content,
> getting stopped when you may lose a totally untracked thing might be
> nicer.
>
> On the other hand, I suspect to many others this finer grained
> control does not give much value while adding more confusion.
>
> I am obviously biased because I am accustomed not to have this
> distinction and accept "checkout -f" as a reasonable way to force
> switching to another branch discarding any and all local
> modifications including untracked new files that get in the way,
> though. But I do not feel strongly enough to say that the behaviour
> and the feature itself is misguided and we should rip it out. As
> long as that "finer grained control" is working in a consistent and
> explainable way, I'd actually vote for fixing the documentation to
> explain how "--discard-changes" is a bit milder than "--force'.
Makes sense. For what it's worth, I don't have much of an opinion one
way or another. I want some way to do what --force currently does, but I
can always use `git checkout` for that if needed.
^ permalink raw reply [flat|nested] 4+ messages in thread