jj icon indicating copy to clipboard operation
jj copied to clipboard

FR: Allow templating the output branche(s) of `jj branch create`

Open estk opened this issue 11 months ago • 14 comments

I would like to be able to create a branch with a name generated from its revset. Basically like jj git push -c but more general and without the push. The idea would be to be able to create topic specific "push" branches.

Solution considered Add a template argument to jj branch create.

jj branch create --evaluate-name-as-template '"foo-" ++ change-id' # Create a branch foo-<id of @>
jj branch create -r main::@ --evaluate-name-as-template '"foo-" ++ change-id'  @ # create branches for set named foo-<id>

Describe alternatives you've considered I suppose branch could have a dedicated option but templates seem like the direction the project is headed.

Edits: Title and arguments as suggested by yuja

estk avatar Mar 08 '24 01:03 estk

jj branch create -T '"foo-" ++ change-id'

Perhaps, it shouldn't be -T <template> (which usually specifies output template, and multiple -T aren't allowed), but a flag --evaluate-name-as-template [<NAME>].. or something like pattern prefix template:<NAME>.

This feature will also apply to jj describe -m <TEMPLATE>. (Here -m "foo: bar" isn't uncommon, so the template: prefix syntax wouldn't be ideal, though.)

yuja avatar Mar 08 '24 07:03 yuja

I like that ergo much better than what I suggested.

Regarding the following, it seems like a useful flag that the jj team may want to discuss within the context of all possible applications in the cli and then decide the best way forward.

This feature will also apply to jj describe -m <TEMPLATE>. (Here -m "foo: bar" isn't uncommon, so the template: prefix syntax wouldn't be ideal, though.)

estk avatar Mar 08 '24 15:03 estk

Ok, so I updated my PR. But it raises a question.

Current Use

One Rev -> Many string branch names

Desired Use

Many Revs -> Many branch names derived from single template

The rub is, how to reconcile the cli use in the unexpected ways:

Case 1

Many Revs -> Many string branch names Perhaps we should error and suggest they use the template option to avoid dup branches?

Case 2

Many Revs -> Many templates Seems fine I guess

My current implementation which is just emergent from the refactor is as follows.

if args.eval_as_template
  for r in revs
    for t in templates
      create_branch(t.format(c));
else
  if r.len() > 1 bail;

Also, I feel I should also add a dry-run option wdyt?

estk avatar Mar 09 '24 04:03 estk

Many Revs -> Many templates

Out of curiosity, what's the purpose of bulk branch creation? Is it a pre step of some other review tooling?

Many Revs -> Many string branch names Perhaps we should error and suggest they use the template option to avoid dup branches?

Error makes sense if there are any branch names to be assigned to different revisions. (And jj branch create should error out anyway if the branch to be created already exists in nested for loop.)

I have no idea about the command line flag. This feature is similar to #3120, and it will probably be implemented as jj describe all:<revset>. The all: prefix unblocks evaluation of multiple revisions. jj describe all:<revset> -m <template> also makes sense feature-wise, but that's odd if all: affected the parsing of -m option. jj describe --batch all:<revset> -m <template> might be okay, but the --batch option is redundant if -m isn't specified.

Also, I feel I should also add a dry-run option wdyt?

Maybe no? It's local operation and can be easily undone. (fwiw, there's a plan of adding global dry-run flag https://github.com/martinvonz/jj/issues/2562#issuecomment-1804348449)

yuja avatar Mar 09 '24 07:03 yuja

The question in my mind is "do you have to write this in each command handler that might use it, or is the "template" parsing done before the command handler is invoked?"

I had a thought that -M could be an enhanced -m for describe, but it doesn't really apply to other commands.

joyously avatar Mar 09 '24 15:03 joyously

The question in my mind is "do you have to write this in each command handler that might use it, or is the "template" parsing done before the command handler is invoked?"

Interesting! I like that idea. If we revive the templater support for string interpolation that @yuja had in a draft PR once, we could implement this feature without an extra flag. Maybe it would look like this:

jj branch create -r <revisions> 'foo-{change-id}'

martinvonz avatar Mar 09 '24 18:03 martinvonz

jj branch create -r 'foo-{change-id}'

It's a bit scary to apply to jj describe -m '..', but should be less invasive than template:.. prefix syntax.

yuja avatar Mar 09 '24 23:03 yuja

If there is concern about accidental templating, would jj describe -m t'..' work?

estk avatar Mar 10 '24 03:03 estk

t'..' is identical to 't..' in shell, and quoted version "t'..'" would be messy.

yuja avatar Mar 10 '24 07:03 yuja

It's a bit scary to apply to jj describe -m '..', but should be less invasive than template:.. prefix syntax.

True, it is a bit scary in that case. We can have different defaults for different commands and let you override with template: or literal:, kind of how we let you override single-revision revsets with all: for some commands.

martinvonz avatar Mar 11 '24 04:03 martinvonz

let you override with template: or literal:, kind of how we let you override single-revision revsets with all: for some commands.

The problem here is that template: blah blah isn't uncommon as a commit description, so we'll need to find another escape syntax which doesn't look like a literal string and can be strictly parsed.

yuja avatar Mar 11 '24 08:03 yuja

Could we use flag modifier options that precede the value arguments?

jj branch -t '"r-" ++ change_id'
jj describe -m 'move change_id field'
jj describe --template --message '"fix bug present in" ++ commit_id(main)'
jj describe -t -m '"fix bug present in" ++ commit_id(main)'
jj describe -tm '"fix bug present in" ++ commit_id(main)'

estk avatar Mar 11 '24 16:03 estk

Could we use flag modifier options that precede the value arguments?

We don't do any unusual flag handling like that yet and I don't want us to start doing it. I don't even know if clap supports that.

martinvonz avatar Mar 11 '24 16:03 martinvonz

Ya I totally understand, I think the name is "position-sensitive flags".

See: https://github.com/clap-rs/clap/pull/5290 https://github.com/clap-rs/clap/discussions/5287 https://docs.rs/clap/latest/clap/_derive/_cookbook/find/index.html

Edit: Here's an example in diesel: https://github.com/diesel-rs/diesel/pull/3796 Looks like they use the clap cli builder.

estk avatar Mar 11 '24 22:03 estk