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.

Sign up for GitHub

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

Jump to bottom

merge_tools: pass Matcher in for interactive use #2129

Merged
merged 1 commit into from
Sep 25, 2023
Merged

merge_tools: pass Matcher in for interactive use #2129

merged 1 commit into from
Sep 25, 2023

Conversation

arxanas
Copy link
Collaborator

@arxanas arxanas commented Aug 22, 2023

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

@arxanas arxanas changed the base branch from arxanas/split-1 to arxanas/difftool-5 August 23, 2023 04:19
@arxanas arxanas force-pushed the arxanas/split-2 branch 2 times, most recently from d1435a2 to dd71103 Compare August 27, 2023 13:49
@arxanas arxanas force-pushed the arxanas/difftool-5 branch from 4e3836e to d3da8c0 Compare August 27, 2023 15:43
@arxanas arxanas force-pushed the arxanas/split-2 branch 2 times, most recently from 40f849a to ec659af Compare August 29, 2023 21:13
@arxanas arxanas force-pushed the arxanas/difftool-5 branch from 63dcbe2 to 99ce2e1 Compare August 30, 2023 08:55
@arxanas arxanas marked this pull request as ready for review August 31, 2023 16:29
@arxanas arxanas force-pushed the arxanas/difftool-5 branch from 99ce2e1 to ea5692a Compare August 31, 2023 16:29
@arxanas arxanas force-pushed the arxanas/split-2 branch 2 times, most recently from 2ef1ab8 to e30461f Compare September 18, 2023 01:35
@ilyagr
Copy link
Collaborator

ilyagr commented Sep 19, 2023

Re commit message: jj split is already interactive. I think you might mean jj commit --interactive PATHs and jj split --interactive PATHs, which don't yet exist.

martinvonz
martinvonz approved these changes Sep 19, 2023
View reviewed changes
Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to keep this waiting, I don't know why I had not approved this earlier

Base automatically changed from arxanas/difftool-5 to main September 21, 2023 01:29
@arxanas
merge_tools: pass Matcher in for interactive use
04d2217 
For `jj split --interactive`, the user will want to select changes from a subset of files. This means that we need to pass the `Matcher` object when materializing the list of changed files. I also updated the parameter lists so that the matcher always immediately follows the tree objects.
@arxanas
Copy link
Collaborator Author

arxanas commented Sep 25, 2023

@ilyagr do you have a suggestion for an alternate commit message?

@ilyagr
Copy link
Collaborator

ilyagr commented Sep 25, 2023 •
edited
Loading

@arxanas My first thought is to replace jj split with jj split <paths> four times (twice in the commit description, once in another commit description, and once in the changelog).

If you don't like that and nothing else comes to mind, feel free to leave as is.

@arxanas
Copy link
Collaborator Author

arxanas commented Sep 25, 2023

@ilyagr ah, I think you meant to comment on #2130 (which I mistakenly merged)? I'll see about updating that one.

@arxanas
Copy link
Collaborator Author

arxanas commented Sep 25, 2023

@ilyagr Okay, so looking back at it, the reason I added --interactive to jj split really is for the no-paths case. That is, I've been running jj split -i with no paths and wanting it to mean the same as jj split. But it has the actually-useful effect of restricting the set of paths to split, which is what most people would probably use it for, so I've updated the documentation/commit message to make that clearer.

@arxanas arxanas merged commit 642ac8c into main Sep 25, 2023
15 checks passed
@arxanas arxanas deleted the arxanas/split-2 branch September 25, 2023 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinvonz martinvonz martinvonz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@arxanas @ilyagr @martinvonz