git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] Allow passing pipes for input pipes to diff --no-index

From
Thomas Guyot <tguyot@gmail.com>
Date
Sep 20, 2020, 12:54 UTC
Message-ID
<f338b63f-fd89-095c-b036-8d548fd2470c@gmail.com>
In-Reply-To
<20200918180239.GA186717@coredump.intra.peff.net>
On 2020-09-18 14:02, Jeff King wrote:
Show 33 quoted lines
> On Fri, Sep 18, 2020 at 10:48:41AM -0700, Junio C Hamano wrote:
> 
>> Jeff King <peff@peff.net> 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@crustytoothpaste.net/
>>>
>>> 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
Previous: Jeff KingNext: Jeff King
Message 10 of 50 in “Allow passing pipes to diff --no-index + bugfix”
  1. Thomas Guyot-SionnestSep 18, 2020
  2. 2/2 Allow passing pipes for input pipes to diff --no-indexThomas Guyot-Sionnest, Sep 18, 2020
  3. Taylor BlauSep 18, 2020
  4. Thomas Guyot-SionnestSep 18, 2020
  5. Jeff KingSep 18, 2020
  6. Jeff KingSep 18, 2020
  7. Thomas Guyot-SionnestSep 18, 2020
  8. Junio C HamanoSep 18, 2020
  9. Jeff KingSep 18, 2020
  10. Thomas GuyotSep 20, 2020
  11. Jeff KingSep 21, 2020
  12. Junio C HamanoSep 21, 2020
  13. Taylor BlauSep 18, 2020
  14. Jeff KingSep 18, 2020
  15. Jeff KingSep 18, 2020
  16. Taylor BlauSep 18, 2020
  17. brian m. carlsonSep 18, 2020
  18. 1/2 diff: Fix modified lines stats with --stat and --numstatThomas Guyot-Sionnest, Sep 18, 2020
  19. Taylor BlauSep 18, 2020
  20. Thomas Guyot-SionnestSep 18, 2020
  21. Jeff KingSep 18, 2020
  22. Thomas Guyot-SionnestSep 18, 2020
  23. Thomas GuyotSep 20, 2020
  24. Jeff KingSep 18, 2020
  25. Thomas Guyot-SionnestSep 18, 2020
  26. Junio C HamanoSep 18, 2020
  27. Johannes SchindelinSep 23, 2020
  28. Junio C HamanoSep 23, 2020
  29. Johannes SchindelinSep 23, 2020
  30. Thomas GuyotSep 24, 2020
  31. diff: Fix modified lines stats with --stat and --numstatThomas Guyot-Sionnest, Sep 24, 2020
  32. diff: Fix modified lines stats with --stat and --numstatThomas Guyot-Sionnest, Sep 24, 2020
  33. Junio C HamanoSep 24, 2020
  34. Thomas GuyotSep 24, 2020
  35. Junio C HamanoSep 24, 2020
  36. Junio C HamanoSep 24, 2020
  37. Johannes SchindelinSep 23, 2020
  38. diff: Fix modified lines stats with --stat and --numstatThomas Guyot-Sionnest, Sep 20, 2020
  39. Taylor BlauSep 20, 2020
  40. Thomas GuyotSep 20, 2020
  41. Junio C HamanoSep 20, 2020
  42. Junio C HamanoSep 20, 2020
  43. Junio C HamanoSep 20, 2020
  44. Junio C HamanoSep 20, 2020
  45. Jeff KingSep 21, 2020
  46. Junio C HamanoSep 21, 2020
  47. Jeff KingSep 21, 2020
  48. Junio C HamanoSep 21, 2020
  49. Junio C HamanoSep 18, 2020
  50. Thomas Guyot-SionnestSep 18, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.