-
Notifications
You must be signed in to change notification settings - Fork 356
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: jj squash
should allow you to preserve the commit even if it was made empty
#3815
Comments
I can relate to having "this command modifies commits. This command modifies files within commits", but I don't see how your example illustrates that. I've always thought that |
This is an interesting idea. I think I like it, though I may have missed something and I'm also curious of other people's opinions. However, I don't like the name
If we go this way (which seems useful and uncontroversial to me as well), I think something like Update: For Update 2: To elaborate on the |
Also, I should mention that... (Update: Nope, they are not the same because of the merge.) ... in the most common case of moving from a child to a parent (Update: in a linear history), |
This was meant to illustrate an example where you might want to keep the metadata associated with a commit. It happened to be what I was doing when I found this problem, but isn't the only time you might want to do so. I can think of ~5-10 different reasons you might want to do so off the top of my head - most are relatively niche, but they do add up. My use case is that I'm working on multiple features at once, and most of the time I want to compile all those features at the same time. To do this I just have a local working copy which is a merge of all the features, work on that the same way you would a squash-based workflow, and then amend the feature I was working on in the merge commit by running
That seems fair. I mainly used
SGTM, I just said the first thing that popped into my head, wasn't fussed on the name.
Ah, I hadn't even considered that! That'd work in my use case (I was working on files that happened to be only modified in one feature). Though the fact that I hadn't even considered that is kind of part of the problem. When I move files to a parent commit, I use squash - I shouldn't have to think about the side effects of such an action, and if there are alternative ways of achieving the same effect without those side effects, and whether those other options have side effects. |
I picked my partner's brains, who's unfamiliar with I think it actually makes a lot of sense for most use cases. If I have the commit chain Where it might start to get confusing, though, is:
I'd be interested to see whether other people feel confused by that, or whether it sounds like a good idea? |
Just in case you don't know this already, I am not sure what I think about the idea about making them distinct in whether they work on whole commits. At first thought, I don't like it, but maybe I'd get used to it. |
Yeah, I'm aware of that. I'm struggling to think of a use case where you actually want amend to modify the commit metadata of either child or parent though (and I can think of a few minor circumstances where it'd be bad to do so). Do you have any? One argument for it is that if we were to go with my original suggestion of "squash only works on commits, and move-patch only works on files", then we would have to choose whether amend would operate on files or commits. If that were the case, then it should definitely be files (otherwise, we couldn't do
I feel like you're thinking about this differently to me. Rather than thinking of it as a "whole commit", I think of it as "some subset of files (possibly all)" vs "all files + commit metadata". My point here is that |
I am not sure whether you have some distinction between "amend" and "squash" in mind; I currently don't, so I'll answer this for "squash". I quite often create temporary commits that are meant to be squashed into other commits later (say, after I make sure that they work the way that I expect them to). I quite like them disappearing on Another related use-case that comes to mind is when I want to squash multiple consecutive commits into one. I don't feel like |
I do have a distinction in mind - to me, the intention of After thinking about it further though, I think my question may be a bit silly. To me, squash and amend are two different commands, and thus those examples you provided to me are squashes, and I can't think of a use case because to me because any use case where you would want to do that is a squash, not an amend. To you, it appears that you don't distinguish, and thus you can see a use case for this. Maybe this is because when I first switched to jj, my translation was Interesting to hear other people's perspectives on this though. To me, the thought of "I need to squish these two commits into one" is quite different from "Update this commit with changes from this other commit", and I think that's probably the right way to think about it, but I'd be open to discussions on this matter if other people think differently. |
Both of those sound to me like "merge", and both sound like hacking the DAG to meet an external criteria. Although that is often the case for developers, maybe it shouldn't be the focus of commands. Isn't the goal of |
Based on discussions both here and in the discord, it sounds like my first option is controversial, and that we should just add an option to the existing squash command to not abandon empty commits.
Could you explain this more? This comment confused me. It seems like a very natural thing to want to move files without commit metadata. And it's definitely not a merge (at least in the
I agree, which is why I find it very annoying that jj destroys your change metadata and creates a new change when I don't want it to. |
Ha ha! Your comment of "squish two into one is different from update one from another" confused me. What I meant was that both are merging or combining changes. I wasn't referring to |
I guess my perspective might be different from yours. I think @mlcui-corp described my thoughts very well in a comment on the discord
It personally doesn't feel like a hack to me that I want to avoid moving metadata. The hack to me feels like the fact that we made "combine two changes, including description" the default, when as @mlcui-corp said, 99% of the time I just want to combine the file changes. I think I can come up with a better example that might help you understand the problem:
Consider that this is something that I'm likely to do without thinking too much about it. I simply make a change to My thoughts is simply that
My proposal would involve
|
+1 for @matts1's comments. I think we should get more thoughts on this from the community in general. My thoughts: Differentiating intent: Agreed that we should differentiate. Using How we differentiate - two commands vs. a new flag: I think two commands would be best. I agree that the current "overloading" of
In both, there would be no need to explain "empty commit abandon" logic, as it wouldn't exist. The interface simplicity would also make the commands much more accessible to new I think the main issue for splitting the two commands would be breaking the habits of existing |
I agree that moving diffs and squashing (or folding) commits are conceptually different, but I'm not sure if adding two separate commands is the right approach. I think the boundary is a bit fuzzy in jj because it's easy to stack up volatile commits (with no description.) I would have to use "squash" to combine them, but I'm only interested in file diffs. Another possible use case is moving some file diffs and commit message without making a commit empty. If the interactive tool supports selecting a commit description in addition to file diffs, this kind of operation might be useful. |
If the user was thinking in terms of files, instead of commits, then we shouldn't abandon the commit, as that can have side effects such as moving branch pointers. Fixes jj-vcs#3815
If the user was thinking in terms of files, instead of commits, then we shouldn't abandon the commit, as that can have side effects such as moving branch pointers. Fixes jj-vcs#3815
If the user was thinking in terms of files, instead of commits, then we shouldn't abandon the commit, as that can have side effects such as moving branch pointers. Fixes jj-vcs#3815
If the user was thinking in terms of files, instead of commits, then we shouldn't abandon the commit, as that can have side effects such as moving branch pointers. Fixes jj-vcs#3815
If the user was thinking in terms of files, instead of commits, then we shouldn't abandon the commit, as that can have side effects such as moving branch pointers. Fixes jj-vcs#3815
If the user was thinking in terms of files, instead of commits, then we shouldn't abandon the commit, as that can have side effects such as moving branch pointers. Fixes jj-vcs#3815
If the user wrote, for example, `jj squash foo.rs`, it'd be a little wierd if we then moved a branch pointer to the parent commit. Fixes jj-vcs#3815
Is your feature request related to a problem? Please describe.
Squash is one command designed for two different semantic use cases. To be honest, this makes me very concerned, as it feels very much like the mistake git made with
checkout
where they needed to, later on, split it intogit switch
andgit restore
.This presents issues when the files that I want to merge just happen to be all the files currently in the commit. For example, consider the following use case:
jj squash --from merge --to A
I would want, in this scenario, to preserve everything about the merge commit:
However, in reality, my merge commit is abandoned, and it attempts to add the description "merge" to the parent commit (I can avoid this second part with
-u
). It then creates a new merge commit with an empty string as the description.Describe the solution you'd like
I think that it's really nice to have a very clear distinction of "this command modifies commits. This command modifies files within commits". My personal preference would be to split
jj squash
into two commands.jj squash
would work on whole commits. It would no longer support--interactive
, or paths (if you wanted that, you would usemove
instead)jj move
would work on files. It wouldn't touch anything related to commits.However, maybe that's a controversial opinion (I'd be interested to see what others think to see whether it is a controversial one). I think a less extreme approach, if people don't like my first suggestion, would be:
jj squash
would get an option--no-abandon
or something to that end, where it wouldn't abandon empty commits.jj move
would be added to the default config as an alias to["squash", --no-abandon"]
.Describe alternatives you've considered
I've described multiple potential approaches above.
Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: