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
Contributor

@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

@samueltardieu samueltardieu force-pushed the split-all-changes-interactively branch from cc8d65c to b5fc7e3 Compare September 26, 2024 18:22
@martinvonz
Copy link
Member

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 samueltardieu force-pushed the split-all-changes-interactively branch from b5fc7e3 to f5abd6d Compare September 26, 2024 19:31
@samueltardieu
Copy link
Contributor 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 force-pushed the split-all-changes-interactively branch from a50cecd to 1cbec40 Compare September 27, 2024 10:56
@samueltardieu samueltardieu merged commit f38c59f into jj-vcs: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