Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
estk opened this issue Mar 8, 2024 · 14 comments
Open

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

estk opened this issue Mar 8, 2024 · 14 comments

Comments

@estk
Copy link

estk commented Mar 8, 2024

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 estk changed the title FR: FR: jj branch create -T Mar 8, 2024
@yuja
Copy link
Collaborator

yuja commented Mar 8, 2024

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.)

@estk
Copy link
Author

estk commented Mar 8, 2024

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 . (Here -m "foo: bar" isn't uncommon, so the template: prefix syntax wouldn't be ideal, though.)

@estk estk changed the title FR: jj branch create -T FR: Allow templating the output branche(s) of jj branch create Mar 8, 2024
@estk
Copy link
Author

estk commented Mar 9, 2024

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?

@yuja
Copy link
Collaborator

yuja commented Mar 9, 2024

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 #2562 (comment))

@joyously
Copy link

joyously commented Mar 9, 2024

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.

@martinvonz
Copy link
Owner

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}'

@yuja
Copy link
Collaborator

yuja commented Mar 9, 2024

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.

@estk
Copy link
Author

estk commented Mar 10, 2024

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

@yuja
Copy link
Collaborator

yuja commented Mar 10, 2024

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

@martinvonz
Copy link
Owner

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.

@yuja
Copy link
Collaborator

yuja commented Mar 11, 2024

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.

@estk
Copy link
Author

estk commented Mar 11, 2024

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)'

@martinvonz
Copy link
Owner

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.

@estk
Copy link
Author

estk commented Mar 11, 2024

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

See:
clap-rs/clap#5290
clap-rs/clap#5287
https://docs.rs/clap/latest/clap/_derive/_cookbook/find/index.html

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants