git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* feedback/idea about "switch -C" force create
@ 2021-06-29 11:11 Martin
  2021-06-29 11:33 ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 4+ messages in thread
From: Martin @ 2021-06-29 11:11 UTC (permalink / raw)
  To: git

First of all, my first post, hope it's the correct place.

I've been using the new git switch for some time, and also I have helped 
people new to git.
When people are new to git, I try to avoid introducing them to commands 
that can loose them commits (such as git reset).

"git switch" however has to be on the list of commands that new user 
have to learn early, but unfortunately when used with "-C" it may cause 
the loss of commits.
I am aware, it is a force option. But I still think it may be better if 
it could emit a warning, or even reject the job.

Reasons:
1) Newcomers may not be aware of the extend of such a force at all. 
Newcomers may not expect loss of commits, on such an elementary command.

2) People aware that it is a "force" may not be aware of the extend of 
the force, because there are either up to 2 actions forced.

Action 1)
The move of the branch is forced.
- That means, the info which commit was on the top of the branch before 
will be lost.
- Also the move may affect push-ability without "force"

Action 2)
The commit may be lost (except for the reflog, but many less experienced 
people do not know that).
This loss is dependent on other factors. It may or may not happen.
Because it does not always happen, people may not expect it.

As a result:
- A user could believe the force is for the effect on the branch, and be 
unaware of the loss of commit
- A user (ever experienced) could opt for the force in the good belief 
that their commits are held by other branches, when maybe they are not.

Therefore I believe, it would be best, if   git -C  branch new-location
would give an error, if this will lose commits.

There could be
- a git config to  toggle this
- an additional command line option to extend the force to drop commits


I would like to know if that idea might in general be acceptable at all.
If so, where it could or should be made as a feature request 
(unfortunately I wont be able to provide a patch myself)


On top, I would propose that the documentation of the current behaviour 
should be made more clear.

https://git-scm.com/docs/git-switch about -C / --force-create
>   Similar to --create except that if <new-branch> already exists, it 
> will be reset to <start-point>. This is a convenient shortcut for:
>
>   $ git branch -f <new-branch>
>   $ git switch <new-branch>
While the word "force" is in the option itself, the description does not 
explain what is forced, or what effects this may have.
Instead it only refers the user to study another option.
I believe the documentation should state directly
- commits currently in part of that branch may be lost [under certain 
circumstances]

and maybe, but less important
- the old location of the branch will be lost



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: feedback/idea about "switch -C" force create
  2021-06-29 11:11 feedback/idea about "switch -C" force create Martin
@ 2021-06-29 11:33 ` Ævar Arnfjörð Bjarmason
  2021-06-29 13:44   ` Martin
  2021-06-29 15:01   ` Martin
  0 siblings, 2 replies; 4+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-06-29 11:33 UTC (permalink / raw)
  To: Martin; +Cc: git


On Tue, Jun 29 2021, Martin wrote:

> First of all, my first post, hope it's the correct place.

Hi Martin. Welcome, and yes you're in the right place.

> I've been using the new git switch for some time, and also I have
> helped people new to git.
> When people are new to git, I try to avoid introducing them to
> commands that can loose them commits (such as git reset).
>
> "git switch" however has to be on the list of commands that new user
> have to learn early, but unfortunately when used with "-C" it may
> cause the loss of commits.
> I am aware, it is a force option. But I still think it may be better
> if it could emit a warning, or even reject the job.
>
> Reasons:
> 1) Newcomers may not be aware of the extend of such a force at
> all. Newcomers may not expect loss of commits, on such an elementary
> command.
>
> 2) People aware that it is a "force" may not be aware of the extend of
> the force, because there are either up to 2 actions forced.
>
> Action 1)
> The move of the branch is forced.
> - That means, the info which commit was on the top of the branch
>   before will be lost.
> - Also the move may affect push-ability without "force"
>
> Action 2)
> The commit may be lost (except for the reflog, but many less
> experienced people do not know that).
> This loss is dependent on other factors. It may or may not happen.
> Because it does not always happen, people may not expect it.
>
> As a result:
> - A user could believe the force is for the effect on the branch, and
>   be unaware of the loss of commit
> - A user (ever experienced) could opt for the force in the good belief
>   that their commits are held by other branches, when maybe they are
>  not.

Makes sense. So basically the users are not using "-C" as some typo for
"-c" (e.g. as a result of using the readline M-u key-combo), but mean
"force", they just don't know what "force" actually does, and lose data.

While I suspect the answer is something like "they had no idea, git
errored, and they tried another option" I'd be interested if you know
what these users were expecting the "force version of -c" to do in this
case & what the use-case is.

E.g. are they using this to (re-)create a local topic branch for an
upstream PR that may or may not have advanced, and they may or may not
have local work they forgot about, so they get in the habit of using it
& sometimes it destroys their data. More below...

> Therefore I believe, it would be best, if   git -C  branch new-location
> would give an error, if this will lose commits.

...so just to clarify, do you mean that forcing it would be OK if you
advance the branch, so in git.git terms you could do:

    git switch -c mynewbranch master # creates new branch
    git switch -C mynewbranch next

Which would be OK since what's in "next" is in "master", But doing the
reverse would error:

    git switch -c mynewbranch next # creates new branch
    git switch -C mynewbranch master

Do you think just a plain "-c" should also error if you "lose" a commit
by committing on a detached head, or that should be treated differently
(and if so, why?). I.e.:

    git checkout master^0
    touch foo && git add foo && git commit -m"foo"
    git switch -c a-branch master

Which gives you something like:
    
    $ git switch -c a-branch master
    Warning: you are leaving 1 commit behind, not connected to
    any of your branches:
    
      ceccd14206 foo
    
    If you want to keep it by creating a new branch, this may be a good time
    to do so with:
    
     git branch <new-branch-name> ceccd14206

I mean, you could pun on that warning for what you're suggesting, but it
seems to me that it would be sensible that they're consistent, no?

I.e. either we don't error on -C and give a similar useful warning (now
we don't say anything), or error on both.

Don't read any of the above as disagreement with your proposal, just
spitballing ideas & edge cases.

> There could be
> - a git config to  toggle this
> - an additional command line option to extend the force to drop commits
>
>
> I would like to know if that idea might in general be acceptable at all.
> If so, where it could or should be made as a feature request
> (unfortunately I wont be able to provide a patch myself)

Ideas are most welcome, unfortunately the git project is almost entirely
limited by who can come up with patches.

I had some more general proposals for the UI of "git switch" recently at
https://lore.kernel.org/git/877dkdwgfe.fsf@evledraar.gmail.com/

None of that steps on the toes of the idea you have explicitly, but it's
a very adjacent area, so I'd be interested to know what you think.

I haven't had the time or desire to write up complete patches for that
idea of mine, so it's gone nowhere thus far. I'm afraid yours will
probably similarly languish unless you or someone else is willing to
pick it up...

> On top, I would propose that the documentation of the current
> behaviour should be made more clear.
>
> https://git-scm.com/docs/git-switch about -C / --force-create
>>   Similar to --create except that if <new-branch> already exists, it
>> will be reset to <start-point>. This is a convenient shortcut for:
>>
>>   $ git branch -f <new-branch>
>>   $ git switch <new-branch>
> While the word "force" is in the option itself, the description does
> not explain what is forced, or what effects this may have.
> Instead it only refers the user to study another option.
> I believe the documentation should state directly
> - commits currently in part of that branch may be lost [under certain
>   circumstances]
>
> and maybe, but less important
> - the old location of the branch will be lost

...realistically unless you provide a patch or someone interested
happens to pick this up (which I'd deem to be not likely) this
observation is likely to just languish in our ML archive.

If the "I won't be able to provide a patch" has to do with not thinking
you have the know-how to do so I and others would be happy to
help.

E.g. in the case of the documentation change you can even do the change
entirely on the github.com/git/git web UI, no C compilation
etc. needed. Our Documentation/SubmittingPatches has more info on the
general process.

Most of the actual work in doing the sort of change you're suggesting is
the leg-work of figuring out the relevant UI edge cases. E.g. I could
write (and could provide you, if you want to run with it) a patch for
"git switch" that dies on "-C" unless "--force-i-really-mean-it" or
whatever is provided. The real work is then going through the test
failures, checking/explaining that each thing you need to change as a
result makes sense etc.

I think your idea of a config option for this sort of thing might make
sense, but really only if it's done in a more holistic way. That's
basically saying "I wouldn't like the small change you want unless it's
a more general feature", but I think for users having just *one* of our
many --force options behave this way would just be more confusing than
not.

E.g. Emacs has a general facility for disabling the training wheels it
comes with by default, certain commands are considered dangerous or
confusing to newbies and disabled by default.

I'd think such a mode would be useful e.g. for this, replacing "git push
--force" with the "--force-with-lease" option etc., but starting on an
end-state where a user who means "--force" eventually needs to tweak a
"yes I mean force" for all the commands they regularly use would be
worse for everyone.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: feedback/idea about "switch -C" force create
  2021-06-29 11:33 ` Ævar Arnfjörð Bjarmason
@ 2021-06-29 13:44   ` Martin
  2021-06-29 15:01   ` Martin
  1 sibling, 0 replies; 4+ messages in thread
From: Martin @ 2021-06-29 13:44 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git

On 29/06/2021 13:33, Ævar Arnfjörð Bjarmason wrote:
>
> Makes sense. So basically the users are not using "-C" as some typo for
> "-c" (e.g. as a result of using the readline M-u key-combo), but mean
> "force", they just don't know what "force" actually does, and lose data.
>
> While I suspect the answer is something like "they had no idea, git
> errored, and they tried another option" I'd be interested if you know
> what these users were expecting the "force version of -c" to do in this
> case & what the use-case is.
There are 2 groups of scenarios.

1)
New users sometime mess up their branches. They may have started on the 
wrong branch and now want to fix the fork point, or the branch diverted 
from the remote and they try to fix the "cannot fast forward". Or they 
created an earlier branch with the same name, and want to re-use it.
Often it is something where a local branch needs rebasing.
In the last case, the earlier branch should be renamed. So that is a 
real user error. But new users are not always aware, how commits are 
hold by branches (I.e. they expect them to stay without needing a branch).

Of course, here the extra force flag needed, only helps, if the error 
explains what is going to happen. So users will not just blindly add the 
force flag.

2)
For the more experienced users, it comes down to oversight. The may 
honestly believe that there was a 2nd branch holding the commits.
I've seen people having a 2nd branch on their feature branch. Maybe 
where the 2nd branch added some commits on top of the feature branch, 
but the feature branch has not yet diverged.

However if you believe your feature branch hasn't diverged -- while in 
reality it has diverged -- then you would expect commits to be hold by 
the 2nd branch.
Therefore you would not expect any loss, from the switch -C.
This could be because you want to push feature~10, and then fast forward 
to your 2nd branch and continue on your local work.
   git switch -C feature  feature~10

And that should mean: I have another branch holding the commits, and I 
use force because I do not care about the exact commit this branch was on.

While, if you really mean: "Move my branch, and drop whatever may get 
dropped" you do
   git switch -f -C feature  new-location
indicating your additional awareness, that there are (or could be) 
commits that will get dropped (you could do -D instead of -f (Drop/Delete))

So you give 2 params that say both "force", if you mean to force 2 actions.


> ...so just to clarify, do you mean that forcing it would be OK if you
> advance the branch, so in git.git terms you could do:
>
>      git switch -c mynewbranch master # creates new branch
>      git switch -C mynewbranch next
>
> Which would be OK since what's in "next" is in "master", But doing the
> reverse would error:
>
>      git switch -c mynewbranch next # creates new branch
>      git switch -C mynewbranch master
Yes going forward (move to children of the current commit) will preserve 
the existing commit, and is fine.

Going to parents, would loose commits. Unless those commits are hold by 
any other branch.
I.e. if an feature branch diverge from some child of the current commit, 
then the feature branch will hold the commit that you move away from.

So it is not needed for the branch that is moved, to keep holding the 
commit. Any other branch does fine too.



> Do you think just a plain "-c" should also error if you "lose" a commit
> by committing on a detached head, or that should be treated differently
> (and if so, why?). I.e.:
>      
This is an interesting idea.

And, yes I think that this should be done too.
At least for "git switch", which being marked as experimental does allow 
for those changes.
Changing "git checkout", may cause problems for users relaying on the 
behaviour.

However, I agree for the reason of treating it equal, and only for that 
reason.

The scenario with the detached commit, is something that an experienced 
user could fall for, if they "forget".

But it is less an issue for newbies. At least, if they are taught to use 
"git switch", and don't use "git checkout" at all.
"git switch" does not let you detach by accident. It requires you to 
specify --detach.
So users should be aware they are detached.

With "git checkout" you can indeed get easily detached, without having 
intended this. (Back in my newby days, when git switch did not even 
exist as an idea, I learned that the hard way)


> I mean, you could pun on that warning for what you're suggesting, but it
> seems to me that it would be sensible that they're consistent, no?
Yes.

> I.e. either we don't error on -C and give a similar useful warning (now
> we don't say anything), or error on both.
Yes.
Adding a warning would be an option too.

I think the error is more sensible though. It makes sure it is not 
overlooked.
And, this is why there are "force" options.

I.e. switching from a detached commit, requires one count of force (to 
loose the commit)
git switch -f some-branch

Recreation of an existing branch, and at the same time loosing commits, 
should require to counts of force
git switch -f -C existing-branch new-location

If force is used, the warning can be reduced to
         $ git switch -f -C existing-branch new-location
         Info: The commit ABC123456 and <n> parents were dropped.

Since the user forced it, recovery info is not needed. But I would not 
object to include a line
        To recover those commits run
          git branch new-name ABC123456


> Don't read any of the above as disagreement with your proposal, just
> spitballing ideas & edge cases.
Absolutely. I did not expect my idea to be pitch perfect.
It's an idea, it needs to grow into a well formulated plan.

>> There could be
>> - a git config to  toggle this

> Ideas are most welcome, unfortunately the git project is almost entirely
> limited by who can come up with patches.
I understand this too. I participate in other open source projects, so those limitations are not new to me.

  
> I had some more general proposals for the UI of "git switch" recently at
> https://lore.kernel.org/git/877dkdwgfe.fsf@evledraar.gmail.com/
>
> None of that steps on the toes of the idea you have explicitly, but it's
> a very adjacent area, so I'd be interested to know what you think.

I will reply in a 2nd email.

  

> If the "I won't be able to provide a patch" has to do with not thinking
> you have the know-how to do so I and others would be happy to
> help.
At the moment it is a time restraint.


> E.g. in the case of the documentation change you can even do the change
> entirely on the github.com/git/git web UI, no C compilation
> etc. needed. Our Documentation/SubmittingPatches has more info on the
> general process.
Making a mental note of it.

> Most of the actual work in doing the sort of change you're suggesting is
> the leg-work of figuring out the relevant UI edge cases. E.g. I could
> write (and could provide you, if you want to run with it) a patch for
> "git switch" that dies on "-C" unless "--force-i-really-mean-it" or
> whatever is provided. The real work is then going through the test
> failures, checking/explaining that each thing you need to change as a
> result makes sense etc.
As I said, it's a not enough time issue.
And C is not my main field. I do not even have a work environment set up 
for it. Not an excuse, just an "needs even more spare time"
Unfortunately it's not likely I will find that time soon, but if I can 
find it, and get to the point were I can set up an environment to run 
the tests, I may come back on this.
It is a slim "if" though.


> I think your idea of a config option for this sort of thing might make
> sense, but really only if it's done in a more holistic way. That's
> basically saying "I wouldn't like the small change you want unless it's
> a more general feature", but I think for users having just *one* of our
> many --force options behave this way would just be more confusing than
> not.
Agreed. But then that part can currently be left out.
It becomes an entire feature of it's own.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: feedback/idea about "switch -C" force create
  2021-06-29 11:33 ` Ævar Arnfjörð Bjarmason
  2021-06-29 13:44   ` Martin
@ 2021-06-29 15:01   ` Martin
  1 sibling, 0 replies; 4+ messages in thread
From: Martin @ 2021-06-29 15:01 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git

On 29/06/2021 13:33, Ævar Arnfjörð Bjarmason wrote:
> I had some more general proposals for the UI of "git switch" recently at
> https://lore.kernel.org/git/877dkdwgfe.fsf@evledraar.gmail.com/
>
> None of that steps on the toes of the idea you have explicitly, but it's
> a very adjacent area, so I'd be interested to know what you think.
>

> I think it's quite confusing that "git switch" doesn't switch to a new
> "doesnotexist" branch on something like:
>
>      git switch doesnotexist
>
> But requires:
>
>      git switch -c doesnotexist

I think that creation should require a parameter.
If not you end up on a non tracking branch, while you might expect to be 
on a local tracking branch.


>   But it really
> should be:
>
>      # -n or -N for --new / --new --force (the latter just in case of a
>      # race, and just for consistency)
>      git switch -n doesnotexist

This is an interesting one. I haven't taken the time to check if there 
are other commands where a letter used as option, translates to a 
different option between different commands.
Using -n would be ok.

BUT it is only halve the issue
> (branch: add a --copy (-c) option to go
> with --move (-m), 2017-06-18) has the knock-on effect that we can't
> mirror the "git branch" UI. I.e. to make "switch" be "branch with
> checkout" for these common cases.
even if -c became -n, there still be plenty of people/scripts who would 
not be aware.

"-c" would have to be disused for a real long time, before I would feel 
comfortable to use it for another purpose.

Some users are stuck for long time on an old git version, depending on 
their OS. When they upgrade they may skip many releases.
So they could easily issue a -c for create, and trigger a copy.

As for "move", this is not the most intuitive name for "rename" anyway. 
(I get it, move the content to the new name, but its also move the old 
name, since that does not remain)
But all good letters are take (branch -r => list remotes)

I can see some sense, in a shortcut for "copy and switch".
But does git switch, need to have move/rename ?

Especially in the one branch only specified version
       git switch -m newname
does the same as
     git branch -m newname
The branch is renamed. There is no actual switch happening. The same 
branch is still checked out, just under new name.

       git switch -m newname  existingbranch
That would be a shortcut for rename, then switch. (not sure if that common)

> I.e. to me the ideal end state would be to deprecate (or at least
> warn/discourage) the "git branch -m" case where it does its own checkout
> (but for nothing else), and to make "git switch" a "branch with
> checkout" with the same -c/-C/-m/-M semantics, just also with a -n/-N
> for "create first".
I never thought as "git switch" as a "git branch" replacement.

I think those 2 are (and should be) distinct entities.
My understanding is, that "git switch" (and restore) are a result of 
the  fact that "git checkout" combined to many functions in one command.

Btw, a general issue...
"git reset" also is a case of "grab all".


> I.e. to me the ideal end state would be to deprecate (or at least
> warn/discourage) the "git branch -m" case where it does its own checkout
> (but for nothing else),
I am missing something?
   git branch -m
does not seem to perform a checkout?

Well, maybe it is depending on how you view it.
     git branch --move  current-name   new-name

If you say, it moves the content of current to new, then yes it does 
checkout new.
But it also does delete current (because the old name no longer exists.)

This is, why I think the whole think is more of a "rename".
But if this is a rename command, then you are still on the same branch, 
only under new name. And then, there was no checkout.


> So at the end of the day you still have to use "git branch" for these
> common (at least for me) operations of copy/move, *and* maintain a
> mental model that "-c" means "xyz" here, but "abc" there.
I think "git branch" should be the main command to deal with branches, 
but it should not do any checkout.
If it currently does "switch" depends on what --move really is.

> The "switch" command also solves the very real problem (and I believe
> this was the main motivation) of not knowing beforehand if "checkout"
> will interpret your "foo" as a file, a branch or whatever.
On top of that, by splitting the different operations that checkout 
performs to new commands (with IMHO easier to remember name) also makes 
it easier to learn.
For a new user checkout just combined to many things.
For a seasoned user this is of course not that much of an issue.

For example  if you give "git checkout" a branch name, then you switch 
to that branch. Ok.
But If you give "git checkout" a branch and a file, you do not switch:  
git checkout master~2 Makefile  (get only the file)

The latter is now
    git restore --source=master~2  Makefile

It is a few chars more to type. But it is much more expressive what it does.
The command name alone makes it clear, this is not "switch".


There are also small improvements, like switch now insists on --detach. 
checkout would be happy to go without it.





^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-29 15:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-29 11:11 feedback/idea about "switch -C" force create Martin
2021-06-29 11:33 ` Ævar Arnfjörð Bjarmason
2021-06-29 13:44   ` Martin
2021-06-29 15:01   ` Martin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).