From: Stefan Sperling Subject: Re: got diff: add support for multiple path arguments To: gameoftrees@openbsd.org Date: Thu, 7 Oct 2021 23:56:37 +0200 On Thu, Oct 07, 2021 at 08:59:31PM +0000, Klemens Nanni wrote: > On Thu, Oct 07, 2021 at 02:18:16PM +0200, Stefan Sperling wrote: > > Make "got diff path1 path2 path3 ..." produce a diff of the > > corresponding paths in a work tree. > > This is so much more useful, so no objection from but, but I'd like to > see got's diff semantics become more similar to git's eventually, simply > because they are so helpful. More on this inline. We can iterate further on top of this patch, no problem. > > This feature has been requested several times already. > > I gave it a try some time ago but failed implementing it another way... > > OK kn > > > fprintf(stderr, "usage: %s diff [-a] [-C number] [-r repository-path] " > > - "[-s] [-w] [object1 object2 | path]\n", getprogname()); > > + "[-s] [-w] [-P] [object1 object2 | path ...]\n", getprogname()); > > So either two objects or some number of paths, but even better would be > > got diff [-aswP] [-r repo] [object1 object2] [--] [path ...] I really don't like Git's abuse of -- to separate revisions from path arguments. There is already the established meaning of "stop processing more options" in getopt and this should not be overloaded. And we still have plenty of free option letters left :) > Diff an arbitrary number of paths in the current worktree against HEAD I am not quite sure what you mean with "HEAD" here. Regular 'got diff' would compare files against the base commit. So I am guessing you mean compare against a tip commit of the current branch, a tip which happens to be newer than the work tree's base commit? What's the use case for diffing local changes against an arbitrary version in the repository? 'cvs diff' can do this, and 'svn diff' can do this, too. Do you actually ever diff your local changes against an older commit, or a newer commit, than you are based on? I have never used this feature as far as I recall. All I've ever needed, in cvs, svn, or git, was diffing work tree files against the base, and diffing arbitrary versions stored in the repository against each other. I don't even see how diffing against a newer commit makes any sense ever. What I would need to make sense of such a situation is a 3-way merge diff, not a patch file. And given that we have local commits, if for some reason you want to send out a diff based on some older commit, there's nothing stopping you from committing what you have in your work tree and generating a diff using the in-repository representation of your changes. Am I missing something? > or, iff two objects are given, between those two commits/branches/etc. > I use this with git all the time as it is helpful to get an overview of > what changed between say two versions, e.g. > > git diff 6.9 7.0 -- sys/net Limiting the scope of a diff between two commits makes a lot of sense. How about: got diff -c 6.9 -c 7.0 sys/net sys/dev/pci sys/dev/ic ... Where -c expects an object ID or a reference which resolves to a commit object, much like many other commands. Use of -c would imply -P, i.e. all non-option arguments are paths. A single -c option would also be supported: got diff -c 6.9 sys/net This could be the same as: got diff -c first-parent-commit-of-6.9 -c 6.9 sys/net Or, we could use a single -c option to represent the strange use case above, and show a diff between sys/net in commit 6.9 and sys/net in the worktree. (This is what 'cvs diff' does when given a single -r option.) The downside is that diffing two adjacent commits now requires two -c arguments, which seems inconvenient. We could add another option letter to the mix to support all of these use cases. Provided we actually want to support them all? > (More precisely, I use that with `git log -p' so I get to browse history > for a specific time frame and for specific files only.) > > > + if (worktree != NULL && (ids[0] == NULL || ids[1] == NULL)) { > > + error = get_worktree_paths_from_argv(&paths, > > + argc, argv, worktree); > > + if (error) > > + goto done; > > + } > > get_worktree_paths_from_argv() should probably ensure a unique list, > i.e. neither `got diff -P a a' nor `got diff -P a ./a' should diff "a" > twice. That's also how git behaves. Not that important and something > for a another diff. Indeed, we should do something about this. We could either throw an error, or only diff a given path once as you suggest. The latter approach is probably easier on people who use this command in shell scripts?