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

split: do not prevent all changes from going into the parent commit, and a fix #4542

Merged

Conversation

samueltardieu
Copy link
Collaborator

@samueltardieu samueltardieu commented Sep 26, 2024

Allowing to put everything into the first commit closes #4533.

Question: should we suggest using jj undo if everything was put into one of the two commits in case the user didn't want to do this? Or, do we want this only if, in addition, the empty commit has an empty description, because there should be no reason to create an empty commit?

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

@martinvonz
Copy link
Owner

Should most of the PR description be moved into the commit description (with s/PR/commit/ or similar)? The question makes sense in the PR description.

@samueltardieu
Copy link
Collaborator Author

samueltardieu commented Sep 26, 2024

Should most of the PR description be moved into the commit description (with s/PR/commit/ or similar)? The question makes sense in the PR description.

It even led me to split the commits (using jj split, this is appropriate).

@samueltardieu samueltardieu changed the title split: do not prevent all changes from going into the parent commit split: do not prevent all changes from going into the parent commit, and a fix Sep 26, 2024
@samueltardieu samueltardieu force-pushed the split-all-changes-interactively branch 2 times, most recently from 30a0235 to a50cecd Compare September 26, 2024 22:07
cli/tests/test_split_command.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
cli/src/commands/split.rs Outdated Show resolved Hide resolved
cli/src/commands/split.rs Show resolved Hide resolved
jj split warning was potentially wrong in both interactive and
non-interactive modes when everything is put into the child commit:

- Non-interactive mode: "The given paths does not match any file:
  PATHS". The message is misleading, as the PATHS given on the command-line
  may match files but not match files containing changes.
- Interactive mode: "The given paths does not match any file: " while
  if possible that no paths were given on the command line.
Let the user select all changes interactively and put them into
the first commit, and create a second commit with the possibility
of preserving the current commit message. This was previously only
possible in non-interactive mode by specifying matching paths, e.g.
".".  In both cases, a warning will be issued indicating that the second
commit is empty.
@samueltardieu samueltardieu merged commit f38c59f into martinvonz:main Sep 27, 2024
18 checks passed
@samueltardieu samueltardieu deleted the split-all-changes-interactively branch September 27, 2024 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split behavior differs when using diff editor and passing path
2 participants