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: Use -p/--path everywhere for path arguments #3809

Open
arxanas opened this issue Jun 1, 2024 · 2 comments
Open

FR: Use -p/--path everywhere for path arguments #3809

arxanas opened this issue Jun 1, 2024 · 2 comments
Labels
polish🪒🐃 Make existing features more convenient and more consistent

Comments

@arxanas
Copy link
Collaborator

arxanas commented Jun 1, 2024

Is your feature request related to a problem? Please describe.

I am constantly running jj log <rev> and then it silently doesn't work/hangs because jj is busy trying to find paths that match <rev> in the commit history. This is a common footgun and we've implemented a few checks to try to detect path arguments that look like revset arguments, but it continues to happen.

There's not a fundamental reason why paths are "more important" than revsets for most commands, so we should stop treating them as a "default" kind of argument for most (if not all) commands.

I believe a lot of the path arguments are positional primarily for historical reasons (namely, Git and Mercurial did it that way).

Describe the solution you'd like

  • In all cases, where positional path arguments are currently accepted, arguments of the form -p/--path should also be accepted.
  • In cases where positional path arguments are currently accepted, we should probably deprecate that and require the user to specify with them with -p/--path.

Describe alternatives you've considered

  • Note that revsets also support querying commits that touch paths, which is a related feature, but not entirely the same. The details about which path was specifically selected carries useful semantic information.
    • For example, git log --patch -- <path> will limit the shown diff to only the parts touching the provided paths. I'm not sure if jj does anything similar at present.
  • One problem with requiring -p is that you can't use the shell's globbing functionality.
    • In some cases, you can write --path={foo,bar,baz} to accomplish a similar workflow (as it will then expand to --path=foo --path=bar --path=baz).
    • I think a --paths (plural) argument that accepts any number of subsequent arguments is justifiable.

Additional context
N/A

@yuja
Copy link
Collaborator

yuja commented Jun 2, 2024

For reference, there's a previous discussion in #2554

@BatmanAoD
Copy link
Collaborator

BatmanAoD commented Sep 30, 2024

For log especially, this behavior was very confusing to me, coming from Git. The only way I'm aware of in Git to filter commits by which paths they've touched is the -p argument, which is actually a git diff flag that just happens to have the desired effect on git log. (This behavior is confusing enough that it resulted in my most popular StackOverflow answer by far.)

If file names are no longer given special treatment for the log command, I have an alternate proposal rather than adding a new flag to make the current behavior explicit: don't permit any flags other than -r/--revision itself to change the log revset. That is, if -r <expr> is given, the logic for determining which refs to include will never depend on any other arguments. (Which is not to say, of course, that the commits included won't depend on any other arguments or on the workspace state; of course --at-operation can always change what commits will be shown.)

To be explicit, rather than having a --path argument on log, the help-string could just mention -r files(foo) as the replacement for the old positional foo behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
polish🪒🐃 Make existing features more convenient and more consistent
Projects
None yet
Development

No branches or pull requests

4 participants