jj icon indicating copy to clipboard operation
jj copied to clipboard

FR: Enhance aliases to run multiple commands with conditional logic

Open kuchta opened this issue 1 year ago • 14 comments

If aliases would be able to run multiple commands with conditional logic, many workflow specific commands (porcelain in git jargon) could be implemented by the user or his/her company.

Many current commands expect some idiomatic workflow which could be implemented as a default aliases if aliases allow for conditional logic and new command eg. is-same-revision <revision> <revision>

This change would externalize such workflow specific commands (and discussions) to user configurable defaults and be potentionaly replaced by user/company preferred workflow...

kuchta avatar May 12 '24 13:05 kuchta

This FR is probably a subset of #3262, as it is along the static/dynamic configuration discussion, which has been brought up many times in Discord.

PhilipMetzger avatar May 17 '24 20:05 PhilipMetzger

I actually just ran across this myself. I would very much like an alias that lets me do jj branch set main -r @- && jj git push -b main. Bonus points for allowing it to take the branch name as an argument or to figure it out from the log.

fowles avatar Jun 03 '24 00:06 fowles

This feels somewhat related to #3575 and #3577.

I think we could do it with minimal changes to the templating language. We'd just have to swap out the builtins for a few different ones, and require the user to create a template returning a List<List<String>>. For example, I could write jj upload as:

[template-aliases]
'stack(x)' = 'surround("mutable() && ::(", ")", x)'

[aliases]
upload = {
   args = {
         "fix": {
             "short": "-f",
             "long": "--fix",
             "type": "bool"
         },
         "lint": {
             "short": "-l",
             "long": "--lint",
             "type": "bool"
         },
         "revision": {
             "short": "-r",
             "long": "--revision",
             "type": "revset"
             "default": "@"
         }
   },
   "positional": True,
   commands = '
      [
          if(args.fix,  ["fix", "-r", stack("opts.revision)]),
          if(args.lint, ["lint", "-r", stack(opts.revision)])
          ["git", "push", "-r", opts.revision] ++ positional
      ]
   '
}

What do people think of this? I personally have a lot of use-cases that I'd be able to solve with something like this.

matts1 avatar Jun 14 '24 03:06 matts1

[template-aliases]
'stack(x)' = 'surround("mutable() && ::(", ")", x)'

[aliases]
upload = {
   args = {
         "fix": {
             "short": "-f",
             "long": "--fix",
             "type": "bool"
         },
         "lint": {
             "short": "-l",
             "long": "--lint",
             "type": "bool"
         },
         "revision": {
             "short": "-r",
             "long": "--revision",
             "type": "revset"
             "default": "@"
         }
   },
   "positional": True,
   commands = '
      [
          if(args.fix,  ["fix", "-r", stack("opts.revision)]),
          if(args.lint, ["lint", "-r", stack(opts.revision)])
          ["git", "push", "-r", opts.revision] ++ positional
      ]
   '
}

My gut feeling is that this is too much as a command alias system (and I would probably want to use general-purpose language than template DSL at this complexity.) That said, I think it's not uncommon to chain commands conditionally (like foo && bar || baz), which can't be expressed well as a list of commands.

yuja avatar Jun 15 '24 08:06 yuja

Yeah, I do agree that it does seem to be pushing the boundaries on what I'd feel comfortable using it for.

I think that extensions could definitely solve this use case pretty well, but they have the opposite problem. A general-purpose language can call the jj api, but it's really overkill for what's essentially a simple string transformation.

My personal opinion is that:

  • Aliases should have more power than they currently do
  • More complex things should be solved with extensions
  • We need to decide how much power is right to give aliases, and at what point we decide "this is sufficiently complex that an extension is required"

How about this? This seems far more simple, inspired by #3873

[aliases]
'upload()' = ["upload", "@"]
'upload(r, *args)' = [
    ["fix", "-r", "$r"],
    # What does "-r" do?
    ["lint", "-r", "$r"],
    # I know the *args isn't valid syntax, but I don't know what the right syntax would be
    ["git", "push", "-r", "$r", "$args"],
]

Alternatively, we could try something a little more python-like

[aliases]
'upload(revision="@", *args)' = [
    ["fix", "-r", "$revision"],
    ["lint", "-r", "$revision"],
    ["git", "push", "-r", "$revision", "$args"]
]

I'm personally thinking that foo && bar || baz may be better to put out of scope, and instead put that in the "requires you to write an extension" category. I think that this adds a lot of power, while keeping the congnitive overhead nice and low, and the syntax simple.

matts1 avatar Jun 17 '24 02:06 matts1

My personal opinion is that: Aliases should have more power than they currently do

  • More complex things should be solved with extensions
  • We need to decide how much power is right to give aliases, and at what point we decide "this is sufficiently complex that an extension is required"

Totally agree.

[aliases]
'upload()' = ["upload", "@"]
'upload(r, *args)' = [
    ["fix", "-r", "$r"],
    # What does "-r" do?
    ["lint", "-r", "$r"],
    # I know the *args isn't valid syntax, but I don't know what the right syntax would be
    ["git", "push", "-r", "$r", "$args"],
]

It looks much manageable (lack of --flag parsing will make things simpler), and I like it.

BTW, the current command aliases are expanded in a loop until converge. I think it's similar to C preprocessor. I'm not sure if the proposed aliases syntax can be compatible with the current substitution logic, and if we make it be evaluated like stacked function call, some user aliases might have to be adjusted.

I'm personally thinking that foo && bar || baz may be better to put out of scope, and instead put that in the "requires you to write an extension" category.

I think we'll at least need to support foo && bar && baz. Perhaps, the commands list will be executed in that way?

yuja avatar Jun 17 '24 10:06 yuja

BTW, the current command aliases are expanded in a loop until converge. I think it's similar to C preprocessor. I'm not sure if the proposed aliases syntax can be compatible with the current substitution logic, and if we make it be evaluated like stacked function call, some user aliases might have to be adjusted.

I think it should be compatible. If you don't use parentheses in the alias name, it would expand to name(*args) = [[..., "$args"]]. For example, foo = ["bar", "baz"] would be expanded to 'foo(*args) = [["bar", "baz", "$args"]].

Then for the substitutions, instead of {"foo": ["bar", "baz"]}, we would have {"upload": {0: [[Format{"upload"}, Format{"@"}]], 1..: [[Format{"fix"}, Format{"-r"}, Format{"%s", 1}], ...]}

I think we'll at least need to support foo && bar && baz ... Perhaps, the commands list will be executed in that way?

Yes, that was my assumption on how they would be executed.

Also sorry, I accidentally clicked "edit" on your comment instead of "quote reply". It should be restored now.

matts1 avatar Jun 17 '24 22:06 matts1

I think it should be compatible. If you don't use parentheses in the alias name, it would expand to name(*args) = [[..., "$args"]]. For example, foo = ["bar", "baz"] would be expanded to 'foo(*args) = [["bar", "baz", "$args"]].

Yeah, I just thought there might be a subtle behavior difference, but I don't have any particular example in mind.

With these aliases:

[aliases]
foo = ["--color", "always", "baz"]
baz = []

in the current cpp-like substitution logic, jj -R repo foo log will be substituted as follows:

-R repo foo log
# subcommand([-R repo foo log]) => foo
# subst(foo)
-R repo --color always baz log
# subcommand([-R repo --color always baz log]) => baz
# subst(baz)
-R repo --color always log
# subcommand([-R repo --color always log]) => log
# return

If it's stacked like normal function calls:

-R repo foo log
# subcommand([-R repo foo log]) => foo
# subst(foo, [log])
        --color always baz log
        # subcommand([--color always baz log]) => baz
        # subst(baz, [log])
                       log
                       # subcommand([log]) => log
                       # return [log]
        # return [--color always log]
# return [-R repo --color always log]

(for this example, the result is the same)

yuja avatar Jun 18 '24 01:06 yuja

I've started working on a prototype, I'll try and send it out this week. It doesn't look too difficult.

matts1 avatar Jun 18 '24 06:06 matts1

Ok, it turned out to be more effort than I thought, but I learnt a lot, and had some ideas to improve it.

I was thinking of using the templating language to solve this. For example:

'upload(r, *args)' = """
    # We need support for lists as a first-class citizen of the templating language. At the moment, you can use lists, but you can't define them.
    command(["fix", "-r", r])
        .then(|r| ["lint", "--revision=" ++ r.commit_id])
        # We'd have to allow ++ to work on lists, or add some way to join two lists together
        .then(|| ["git", "push", "-r", r.commit_id] ++ args])
"""

This would allow us to solve two things:

Firstly, we could chain foo && bar || baz

'x()' = """
    command(["foo"]).then(|| ["bar"]).or(["baz"])
"""

Secondly, we can pipe the output of previous commands onto the current command. For example, duplicate would "return" the new commit:

'duplicate_onto_head(r)' = """
    command(["duplicate", "-r", r])
        .then(|duplicate| ["rebase", "-s", duplicate.commit_id, "-d", "@"])
""" 

What do people think of this? It seems to add some complexity, but it feels to me like it strikes the right balance between complexity and power. I also think that although it adds some complexity, using explicit functions instead of just lists of lists makes it more obvious.

matts1 avatar Jun 26 '24 21:06 matts1

I like your earlier proposal better because it was relatively simple. I'm a bit worried that if we try to add something like your most recent proposal, it's going to be a slippery slope to writing our own shell script language.

martinvonz avatar Jun 26 '24 22:06 martinvonz

That seems pretty fair to me (it also simplifies implementation a lot). The question I have left is how we want to do variable formatting. My current thoughts are:

  • Similar to python format strings:
    • "{foo}" for the variable foo
    • "{{foo}}" for the literal "{foo}"
    • "--foo={foo}" would be valid
  • Suppose we had 'command(*args)' = ..., and we ran jj command a b c:
    • Do we allow --foo={args} (["foo", "--foo={args}"] -> ["foo", "--foo=a", "--foo=b", "--foo=c"])
    • Do we allow {args} when not at the end (["foo", "{args}", "bar"] -> ["foo", "a", "b", "c", "bar"])?

matts1 avatar Jun 27 '24 00:06 matts1

  • Suppose we had 'command(*args)' = ..., and we ran jj command a b c:
    • Do we allow --foo={args} (["foo", "--foo={args}"] -> ["foo", "--foo=a", "--foo=b", "--foo=c"])
    • Do we allow {args} when not at the end (["foo", "{args}", "bar"] -> ["foo", "a", "b", "c", "bar"])?

If we don't need the former, I'll just special case "$args" (just like "$@" in sh.)

  • "$args" -> "$args[0]", "$args[1]", ..
  • "foo=$args" -> error?

BTW, I prefer $x than {x} because we already use dollar sign in merge-tools configuration.

yuja avatar Jun 27 '24 01:06 yuja

I also dislike the new proposal and also want to have a slimmed down syntax. But your new proposal probably belongs to the conversation in #3262.

PhilipMetzger avatar Jun 27 '24 16:06 PhilipMetzger

What is the benefit of this approach over the much simpler idea of jj util exec? The discussion here is already pushing against too much complexity, so whatever becomes the agreed-upon tradeoff between complexity and flexibility is going to be much less powerfull than simply shelling out to a user-provided program / script.

jj util exec was dicussed in https://github.com/martinvonz/jj/issues/3001 and is already implemented in https://github.com/martinvonz/jj/pull/4759.

I'm tried to check the discussion if my question was already discussed, this is the closest I found:

A general-purpose language can call the jj api, but it's really overkill for what's essentially a simple string transformation.

I don't quite follow this. People who are using jj are already using a shell language that they are familiar with. Telling them to just write that same language for more complicated aliases seems much simpler than inventing some new systems that users have to learn and maintainers have to support.

senekor avatar Nov 04 '24 18:11 senekor

What is the benefit of this approach over the much simpler idea of jj util exec?

I think supporting functions in aliases (similar to how we already do it in revsets-aliases / template-aliases) might not be a bad idea -- function arguments would make chaining commands with jj util exec easier and could act as documentation (aside: maybe util exec's help could be overridden with these and a custom help blob).

For example, I want to create an "archive" command to prefix a commit's description with "(jj archive) " -- of course, you could always write a shell script to do this, but with functions in aliases, you could also do something like:

'archive(revision)' = [
	'util', 'exec', 'sh', '-c',
	'''jj log --no-graph -T '"(jj archive) " ++ description' -r $revision | jj describe --stdin $revision''']

avamsi avatar Dec 08 '24 17:12 avamsi

One issue I'm noticing in your example is that you're using $revision to access the argument, but that's shell syntax. So you've got a conflict there. Of course, you can use any interpreter with jj util exec, e.g. JS, Python, even Rust. Creating an argument substitution syntax that's a compatible superset of all of these seems difficult at best.

This is in contrast to your comparison with revsets-aliases / template-aliases, where we already control the complete syntax ourselves.

function arguments would make chaining commands with jj util exec easier

What do you mean by that? How is it easier than using $1 in bash?

senekor avatar Dec 08 '24 18:12 senekor

What do you mean by that? How is it easier than using $1 in bash?

I should RTFM -- I didn't realize positional arguments work with jj util exec 🙃.

avamsi avatar Dec 08 '24 19:12 avamsi

Fyi I also recently wrote custom completions (fish) for one of my script-based aliases. That worked without issues on top of the new dynamic completions. Makes it feel even more like a native subcommand.

senekor avatar Dec 08 '24 19:12 senekor

Fyi I also recently wrote custom completions (fish) for one of my script-based aliases. That worked without issues on top of the new dynamic completions. Makes it feel even more like a native subcommand.

Would you have an example, so we can have a better idea of what it means to add completion to an alias?

The lack of completion is something that I'm missing when I write aliases. I hope there is an easy and powerful enough solution to complete a revset expression!

glehmann avatar Sep 06 '25 19:09 glehmann

I use fish, so here are my fish completions. They are stored in ~/.config/fish/completions/jj.fish. That way they are lazily loaded the first time I run jj in a fish sessions. That also overrides the builtin fish completions for jj, which means I have to activate the normal completions explicitly.

COMPLETE=fish jj | source

# complete `bookmark create` with push prefix
complete --command jj \
    --condition "__fish_seen_subcommand_from b bookmark; and __fish_seen_subcommand_from c create" \
    --arguments "(jj config get templates.git_push_bookmark | cut --delimiter '\"' --fields 2)"

# complete `cob` with bookmark names
complete --command jj \
    --condition "__fish_seen_subcommand_from cob" \
    --arguments "(jj bookmark list --revisions 'fork_point(@--)::@' --template 'if(!remote, name ++ \"\\n\")')"

I like to create named bookmarks with the same prefix as auto-generated ones, so I complete that for jj bookmark create. Then I have a alias cob ("commit onto branch") for which I complete the names of bookmarks in my current megamerge.

Not much to it, just regular shell completions written by hand.

senekor avatar Sep 06 '25 19:09 senekor

Maybe we should consider this FR as completed, now that jj util exec has shipped?

senekor avatar Sep 06 '25 19:09 senekor

Maybe we should consider this FR as completed, now that jj util exec has shipped?

Not really since that thing is and stays a hack. Supporting it natively is the correct way to do it, see Matt's PR (https://github.com/jj-vcs/jj/pull/4605). Yuya agrees on this here https://github.com/jj-vcs/jj/issues/7440#issuecomment-3263662312.

PhilipMetzger avatar Sep 07 '25 13:09 PhilipMetzger