Implement #2475

[noahmayr]

Implementation for #2475

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 tests to cover my changes

[yuja]

Regarding #2475, I think the default revset can be turned off if any path arguments are specified.

jj log            # use default revset
jj log -rREV      # REV
jj log PATH       # file(PATH)
jj log -rREV PATH # REV & file(PATH)

It implies that the PATH and REV arguments are basically the same things (with a bit different representations.) WDYT?

[joyously]

Regarding # 2475, I think the default revset can be turned off if any path arguments are specified.

jj log            # use default revset
jj log -rREV      # REV
jj log PATH       # file(PATH)
jj log -rREV PATH # REV & file(PATH)

What is the special case? If I do

jj log -r 'author(yuya)'
jj log CHANGELOG.md
jj log -r 'author(yuya)' CHANGELOG.md

I should get 3 different results.

[yuja]

jj log            # use default revset
jj log -rREV      # REV
jj log PATH       # file(PATH)
jj log -rREV PATH # REV & file(PATH)

What is the special case?

Special case exists only for jj log (without filtering options)?

The current definition is: -r defaults to revsets.log
So jj log PATH means jj log -r<revsets.log> PATH.

My proposal is: revsets.log is used only if no filtering options (-r or PATH) are specified.

[joyously]

My proposal is: revsets.log is used only if no filtering options (-r or PATH) are specified.

Without knowing the code, it seems that the revset is needed (as in the bare jj log).
If the default can't be set in the config file, then I guess you don't need it when there is a path.

[martinvonz]

I agree that it seems better to not use the default revset if any paths were passed. I think that's almost always what I want.

[noahmayr]

Regarding #2475, I think the default revset can be turned off if any path arguments are specified.

jj log            # use default revset
jj log -rREV      # REV
jj log PATH       # file(PATH)
jj log -rREV PATH # REV & file(PATH)

It implies that the PATH and REV arguments are basically the same things (with a bit different representations.) WDYT?

I agree that it seems better to not use the default revset if any paths were passed. I think that's almost always what I want.

I added another commit which changes the behavior to reflect this. If it looks good to you I'll restructure the changes

[yuja]

   // if no args are passed, use default revset
   // if only paths are passed, filter by paths
   // if only a revset is passed, use the revset

These sound good to me, thanks.

   // if paths and revset are passed, use both but try dropping the revset if the
   // paths are not contained in it

Meh, no. If the user specified both, it's clear that they want to filter by both revset and paths.

BTW, I've changed workspace_command.*revset() APIs recently. We might need to make attach_revset_evaluator() public so the "only paths" case can be started from RevsetExpresssion::all().

[yuja]

LGTM, thanks. Can you squash these patches? We usually include doc/tests changes in the commit which introduce the new behavior.

[yuja]

thanks!

[ilyagr]

Also, thank you!

[martinvonz]

Thanks!

[ilyagr]

I don't think you needed to wait for my approval, but just in case.

[noahmayr]

I don't think you needed to wait for my approval, but just in case.

I'm not allowed to merge though, but judging by the fact that my name just turned green on discord I assume someone is already working on that :)

[martinvonz]

I'm not allowed to merge though, but judging by the fact that my name just turned green on discord I assume someone is already working on that :)

I actually thought I already had done that. Done now anyway.