On 2020-09-18 14:02, Jeff King wrote: > On Fri, Sep 18, 2020 at 10:48:41AM -0700, Junio C Hamano wrote: > >> Jeff King <peff@xxxxxxxx> writes: >> >>> Getting back to the overall feature, this is definitely something that >>> has come up before. The last I know of is: >>> >>> https://lore.kernel.org/git/20181220002610.43832-1-sandals@xxxxxxxxxxxxxxxxxxxx/ >>> >>> which everybody seemed to like the direction of; I suspect the original >>> author (cc'd) just never got around to it again. Compared to this >>> approach, it uses a command-line option to avoid dereferencing symlinks. >>> That puts an extra burden on the caller to pass the option, but it's way >>> less magical; you could drop all of the "does this look like a symlink >>> to a pipe" heuristics. It would also be much easier to test. ;) >> >> Yes, I do remember liking the approach very much and wanted to take >> it once the "do not dereference symlinks everywhere---just limit it >> to what was given from the command line" was done. >> >> To be quite honest, I think "git diff --no-index A B" should >> unconditionally dereference A and/or B if they are symlinks, whether >> they are symlinks to pipes, regular files or directories, and >> otherwise treat symlinks in A and B the same way as "git diff" if A >> and B are directories. But that is a design guideline that becomes >> needed only after we start resurrecting Brian's effort, not with >> these patches that started this thread. > > Yeah, I think I'd be fine with that approach, too. It makes "git diff > --no-index" more like other tools out of the box. And if we took brian's > patch first, then we'd just be flipping its default, and the option it > adds would give an easy escape hatch for somebody who really wants to > diff two maybe-symlinks. Considering the issue with MacOS I'm starting to think the best solution is to not use any heuristic and read passed-in files directly. That said, I don't think it makes much change either way (if I resurrect Brian's patch is will probably end up being a hybrid between the two as both read the pipe at the same place and my approach was simpler further down). I'm not sure which way I prefer to start first - will you accept a patch that reads passed in files as-is if I I start with this one? In the mean time I will submit the first patch fixed) as a standalone one. Regards, Thomas
