martinvonz / jj

A Git-compatible VCS that is both simple and powerful
https://martinvonz.github.io/jj/
Apache License 2.0
8.32k stars 284 forks source link

cli: branch: make "set" do upsert as before #3932

Closed yuja closed 3 months ago

yuja commented 3 months ago

3584

Checklist

If applicable:

arxanas commented 3 months ago

Hmm, when I read this PR title (having forgotten about the previous discussion), I thought that --allow-new must be related to jj new somehow. Not sure if it's the best name given that we accidentally stole the meaning of "new" in a general context 🫤. I think --allow-create would be more accurate, but it's wordy.

yuja commented 3 months ago

I thought that --allow-new must be related to jj new somehow. Not sure if it's the best name given that we accidentally stole the meaning of "new" in a general context 🫤. I think --allow-create would be more accurate, but it's wordy.

Hmm, I thought --allow-new is preferred, but I think --allow-create is also good. Some people including me were against --create because set --create doesn't always "create", but I don't see any argument against --allow-create (or --allow-new.)

ilyagr commented 3 months ago

I'm wondering whether we should just be more aggressive here and let set create branches immediately.

For example, for a release or two, we can print a warning when jj branch set creates a branch and suggest jj branch move if that's not what people wanted. There could be an option to disable the warning.


I also agree with https://github.com/martinvonz/jj/pull/3932#issuecomment-2181537160, --allow-create sounds better to me. new is becoming a loaded word in jj, and while we could use it here if we had to, we don't have to.

ilyagr commented 3 months ago

Perhaps I'm not following what your plan is. I guess my concern is that if we merely create the option now and make it the default later, then it feels like the change would be just as breaking at that later point as it would be now if we made the change without the option. What is the option's purpose in your mind?

yuja commented 3 months ago

My plan was:

  1. add --allow-new
  2. add warning if set moved existing branch (suggesting branch move instead
  3. make --allow-new the default

For example, for a release or two, we can print a warning when jj branch set creates a branch and suggest jj branch move if that's not what people wanted.

That sounds also good, and I like the idea.

I also agree with #3932 (comment), --allow-create sounds better to me.

Okay, there are two votes, so let's rename then.

yuja commented 3 months ago

we can print a warning when jj branch set creates a branch and suggest jj branch move

Implemented this as it seemed simpler.