jj icon indicating copy to clipboard operation
jj copied to clipboard

[feature request] Add a `jj range-diff` command

Open chooglen opened this issue 3 years ago • 6 comments

Analogous to git range-diff, this command would visualize changes in a revset , not just single commits (obslog/interdiff has that covered).

The primary use case for this would be to see how a revset has changed over multiple operations (e.g. i'm rewriting v1 -> v2 and want to see what I've changed) and not to compare two arbitrary revsets at the same operation.

chooglen avatar Aug 29 '22 18:08 chooglen

Interesting, git range-diff is very different from what I expected. I expected it to be kind of a generic interdiff that would show the content-changes between the two ranges of commits. I.e., I expected git range-diff foo1..foo2 bar1..bar2 to do this:

$ git checkout bar2
$ git rebase --onto foo1^ bar1^
$ git diff foo2 HEAD

(except that it would all be done temporarily without affecting the repo or the working copy)

I think that would be a natural extension of the jj interdiff I suggested in #23. So instead of its arguments being two revsets that resolve to single revisions, the revsets would be allowed to resolve to many revisions. I'm not sure what restrictions we'd want to put on them (shouldn't need to be contiguous, but maybe at least linear?).

martinvonz avatar Aug 29 '22 22:08 martinvonz

https://stackoverflow.com/questions/52323008/can-somebody-explain-the-usage-of-git-range-diff explains some usecases for git range-diff (in case there are others like me who don't know what it's for). @chooglen, do you think the generalized interdiff would address those usecases for you? An advantage of interdiff is that it shows you the actual changes.

martinvonz avatar Aug 29 '22 22:08 martinvonz

So I'm basically wondering if git range-diff works the way it does simply because it is hard to do a temporary rebase like that with Git (especially before ort-merge, but even after that if there are conflicts).

martinvonz avatar Aug 29 '22 23:08 martinvonz

Applying the changes and diffing them will be much better, I think. Generalized interdiff sounds good, and as a first pass we could match commits based on change id. This should be enough to visualize revset evolution across operations (which is the primary use case anyway).

I doubt we need to be able to diff arbitrary ranges/revsets like git range-diff does. git range-diff doesn't even do that good a job of that IMO, since it just diffs patches against each other, which results in some absurdly unreadable output, e.g. [pulling a random example] off the git mailing list.

     +@@ builtin/bisect--helper.c: static enum bisect_error bisect_auto_next(const char *prefix)
     + 	return bisect_next(prefix);
     + }
     + 
     +-static enum bisect_error bisect_start(const char **argv, int argc)
     ++static enum bisect_error bisect_start(int argc, const char **argv)
     + {
     + 	int no_checkout = 0;
     + 	int first_parent_only = 0;
     +@@ builtin/bisect--helper.c: static int bisect_autostart(void)
     + 	yesno = git_prompt(_("Do you want me to do it for you "
     + 			     "[Y/n]? "), PROMPT_ECHO);
     + 	res = tolower(*yesno) == 'n' ?
     +-		-1 : bisect_start(empty_strvec, 0);
     ++		-1 : bisect_start(0, empty_strvec);
     + 
     + 	return res;
     + }
     + 
     +-static enum bisect_error bisect_state(const char **argv,
     +-				      int argc)
     ++static enum bisect_error bisect_state(int argc, const char **argv)
     + {
     + 	const char *state;
     + 	int i, verify_expected = 1;
     +@@ builtin/bisect--helper.c: static int process_replay_line(struct strbuf *line)
     + 		struct strvec argv = STRVEC_INIT;
     + 		int res;
     + 		sq_dequote_to_strvec(rev, &argv);
     +-		res = bisect_start(argv.v, argv.nr);
     ++		res = bisect_start(argv.nr, argv.v);
     + 		strvec_clear(&argv);
     + 		return res;
     + 	}

So I'm basically wondering if git range-diff works the way it does simply because it is hard to do a temporary rebase like that with Git (especially before ort-merge, but even after that if there are conflicts).

That might be part of it, but in practice, the range-diff tends to be based on the same base anyway so it doesn't matter so much. Another possibility is that diffing the actual commits just produces unhelpful output, e.g. if you diff B1 and B2 in:

  (rewrite)
B1    ->   B2
|
A1    ->   A2

you'd also end up showing the diff from A1 to A2, but what we care about is the diff between (B1 - A1) and (B2 - A2).

chooglen avatar Aug 29 '22 23:08 chooglen

That might be part of it, but in practice, the range-diff tends to be based on the same base anyway so it doesn't matter so much.

That might be true on the Git ML, but is it true elsewhere? I personally quite often rebase PRs (even though I know GitHub deals poorly with it).

Another possibility is that diffing the actual commits just produces unhelpful output, e.g. if you diff B1 and B2 in:

  (rewrite)
B1    ->   B2
|
A1    ->   A2

you'd also end up showing the diff from A1 to A2, but what we care about is the diff between (B1 - A1) and (B2 - A2).

If jj interdiff B1 B2 does a temporary rebase of B1 onto A2 and then diff from B1' to B2 (which is how I imagine it would work), then the diff between A1 to A2 shouldn't show up, right?

martinvonz avatar Aug 29 '22 23:08 martinvonz

That might be true on the Git ML, but is it true elsewhere? I personally quite often rebase PRs (even though I know GitHub deals poorly with it).

True. I guess this is useful for describing why git range-diff is the way it is, but not what it should be.

If jj interdiff B1 B2 does a temporary rebase of B1 onto A2 and then diff from B1' to B2 (which is how I imagine it would work), then the diff between A1 to A2 shouldn't show up, right?

Actually, yeah that should work and it doesn't seem that hard to do.

chooglen avatar Aug 30 '22 17:08 chooglen