cli: -G as short version of --no-graph

[glehmann]

To be consistent with jj log -G.

Checklist

If applicable:

• I have updated CHANGELOG.md
• I have updated the documentation (README.md, docs/, demos/)
• I have updated the config schema (cli/src/config-schema.json)
• I have added/updated tests to cover my changes

[martinvonz]

Feel like adding it to jj evolog too?

[martinvonz]

... and to jj op diff/log/show too? Try grepping for no_graph

[9999years]

I think it would be very valuable to have jj log -G mean the same thing as git log -G (performing a grep on the diffs), but I'm open to feedback.

Do you use -G often? I don't think I've ever used it personally, but if it's a cornerstone of some workflows then it's a trickier proposition.

See:

• `git log -G` / `git log -S` for `jj log` #7678

[ilyagr]

I haven't realized -G conflicts with Git's -G, thanks for bringing that up! Still, IMO it's OK to use it for another purpose in jj. I'm hoping confusing the two will lead to an immediate error, since Git's -G takes a parameter (the string to search for) and ours does not.

This goes together with my understanding that git log's choice of -G and -S for searching inside the commit diffs is somewhat arbitrary. I guess it's something like grep and search, but I find it hard to remember which is which; if anything the --pickaxe-regex option is more memorable to me.

In jj, revset language expressions seem more appropriate for this functionality. Even if we add CLI flags, they'd probably have longer names.

[9999years]

I guess it's something like grep and search, but I find it hard to remember which is which

Agreed. But I think having this functionality available from short options is important; when running a number of searches in sequence to find a specific commit, it's pretty annoying to go from jj log -r '::@ & diff_contains(regex:"foo")' to jj log -r '::@ & diff_contains(regex:"bar")' due to the shell escaping, nested quotes, and parenthesis required for the revset syntax (compare to git log -G foo and git log -G bar). I really like the revset language, but it's pretty cumbersome for tasks like this.

I do think this functionality is important enough to warrant a short flag for it, and I think that it would be slightly nicer, everything else equal, if the short flag were the same between git log and jj log. But I'm happy to learn a different option name, so I don't want to die on this hill.

Do you end up using -G for --no-graph frequently? I'm very curious about this.

[ilyagr]

Do you end up using -G for --no-graph frequently? I'm very curious about this.

It's only just been added in #7639. (So, now is a good time to discuss whether to keep it!)

I expect to use it mainly when doing some automated processing on commits, e.g. jj log -r :: -G -T_ | wc or jj log -r :: -G -Tcommit_id | fzf.

You make some good points. I'm currently a bit under the weather, so I don't promise to think much more deeply right now about what the best compromise would be, but hopefully others are thinking about it as well.

[glehmann]

It's confusing that git only provides a short option name. What would be a good (long) name for these options, without looking at what git already did?

We could also say we go for another short name for --no-graph, like -g, but that would be a bit wasted if we go with something else for the search options :)

[martinvonz]

I don't think it's worth testing the short option. Since it relies on clap, there's no logic in our code that can go wrong. It seems very unlikely to regress. I don't think it's worth spending CPU testing it all the time. I don't know if others agree.

(I think the test you added for --no-graph in test_operations.rs is useful, however.)

[glehmann]

I don't think it's worth testing the short option. Since it relies on clap, there's no logic in our code that can go wrong. It seems very unlikely to regress. I don't think it's worth spending CPU testing it all the time. I don't know if others agree.

That and the cli/tests/[email protected] that lists the short options, so we would see if a short option is removed or modified by mistake.
I'll remove the tests.

[glehmann]

I'll remove the tests.

Done!

[martinvonz]

Thanks

[glehmann]

@ilyagr @9999years Do you want to continue the discussion on the flag name in this PR?

[joyously]

Are there any proposed standards for short flags or for --no- flags? Maybe we should make a standard and follow it.

[ilyagr]

TLDR: I'm personally quite happy with this as is. I'm not 100% sure we have a full consensus on this. @9999years, how do you feel now (and in light of my thoughts below)?

Do you want to continue the discussion on the flag name in this PR?

I am personally happy with this PR as is. In my mind, -G is a good name for --no-graph, and not a great name for something related to diff_contains.

One possibility for the latter would be to have flags like --modifies-text <string-pattern>, --adds-text <string-pattern>, --removes-text. For shorter versions, we could have --text for the first one, --adds for the second ones, --removes for the last one.

Even if that's not great, I'm hoping we can come up with something better than -G and -S. Or we could use -S as short for --search-diffs.

Are there any proposed standards for short flags or for --no- flags?

In my mind, minus-capital-letter is a decent standard for this. We also often use these to suppress warnings or errors (e.g. -B for --allow-backwards, -N for --allow-new).

However, we don't currently do this for any other --no- flags specifically, so this PR and #7639 actively create/strengthen such a convention.

[9999years]

It's confusing that git only provides a short option name. What would be a good (long) name for these options, without looking at what git already did?

Agreed. I suspect they had --grep for matching on the description already and just ... punted? I find -G easy to remember by analogy to grep, but I'm not sure how universal that is. (I can never remember -S for this reason.)

I would propose --grep, except it would do something different than what git does with the same option, which is not ideal (and different in a way that a user might not notice at first).

Perhaps simply --diff-contains, to match the revset function. But I'm not sure what I would want the short option to be. -S for "search"? -C for "contains"? I'm of course partial to -G for "grep" — all else equal, it's nice to respect the muscle memory of Git users.

[9999years]

(As an aside, it's a bit odd to have diff_contains but not e.g. description_contains.)

[9999years]

One possibility for the latter would be to have flags like --modifies-text <string-pattern>, --adds-text <string-pattern>, --removes-text. For shorter versions, we could have --text for the first one, --adds for the second ones, --removes for the last one.

I think we should make it explicit that these operate on the diff and not the description, e.g. --modifies-diff. Although I'm not sure what you would do for -G; --adds-or-removes-diff and --changes-occurrences-diff are both extremely cumbersome!

It's a tricky design space — I really do think that the revset functions are "the right way" to do this, but the quoting they introduce makes for poor ergonomics on the command-line. But then expressing all the options we want on the command-line introduces a combinatorial explosion of search subjects and modes.

[9999years]

I've also opened an issue (rather than the discussion I opened previously) for this functionality:

• FR: Diff/description searching options for jj log (git log -G, -S) #7730

[joyously]

Sort of related: FR: --diff-filter equivalent in jj diff
maybe a mixture for the search?