Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Patch applies fake diffs from commit messages

samizdat.dev

Post reply on HN
Print view View original

21–30 of 43 posts

Re: Patch applies fake diffs from commit messages

#21

Post by antiframe »

Cpoll wrote: post #19
yjftsjthsd-h wrote: post #16

Earlier quoted context omitted.

Alright, allow me to disambiguate in your preferred format. Patch is perfect. Ambiguity is good. There are no better formats for conveying patches. Patch files are readable by humans. Being readble by humans is useful. XML is painful for humans to read and write. JSON is painful for humans to read and write. JSON or XML would actually fix this problem in the format. The patch format could be improved. Formats should…

That's really not that bad, especially with indentation and color coding. You're kind of cheating by putting it into HN, which is terrible for code. > XML is painful for humans to read and write. Speaking of claims no-one made; no-one's talking about writing patch files by hand.

If that's good enough to be human readable than patch is even better.

People do write patch files be hand.

Re: Patch applies fake diffs from commit messages

#24

Post by wnoise »

antiframe wrote: post #21
Cpoll wrote: post #19

Earlier quoted context omitted.

That's really not that bad, especially with indentation and color coding. You're kind of cheating by putting it into HN, which is terrible for code. > XML is painful for humans to read and write. Speaking of claims no-one made; no-one's talking about writing patch files by hand.

If that's good enough to be human readable than patch is even better. People do write patch files be hand.

More commonly, edit them.

Re: Patch applies fake diffs from commit messages

#25

Post by jolmg »

jolmg wrote: post #18

> It matters (to me) because `wget`/`curl` plus `patch` is not some exotic lab setup. If the point is to be able to do `curl https://...deadbeef.patch | patch -p1`, you can just change the extension provided to Github from `.patch` to `.diff`. That way, it just includes the hunks. E.g. https://github.com/torvalds/linux/commit/dca922e019dd758b4c1... I don't see it as a problem with the email format, because I can't im…

I've changed my mind regarding the email format not being a problem. I was thinking of emailed git commits as this ad-hoc thing, but I forgot that `git-format-patch` and `git-am` exist. It's not just an incompatibility between them and `patch`. If you have a line `---` in your git commit message, `git-format-patch` will not somehow escape it, resulting in git-am truncating your git commit message. The email commit format is kinda bad. Github `.patch` exports are just being compatible with `git-am`, so I don't think it's a bug with them.

This is Re:

> I do not yet know whether the bug belongs to GNU patch, GitHub’s .patch export, or the broader patch-format contract.

I don't think this is a problem with GNU patch or the patch format per se, just the emailed commit format. I think the patch format's good because it allows it to be embedded in other texts and also allows comments or extended syntax between hunks. For example, the lines

  diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
  index 15ba592236e845..725a49a0eee72e 100644
not being part of the hunk and probably being seen as a comment by `patch`.

Simply changing `git-am` to accept commit messages being indented in the email format would allow Github fix the issue of their .patch exports being incompatible with `patch`, in addition to fixing that bug about commit messages being truncated when they have a `---` line.

Re: Patch applies fake diffs from commit messages

#28

Post by d--b »

This looks like a feature.

Not sure why somebody thought it was a good idea. There probably was a use case at some point where it seemed smart to allow for things like this. Maybe it’s for CI, but I really can’t think of why.

But I really doubt this could be a bug. I mean why would patch even need to read the commit message if it wasn’t to scan it for diff?

Re: Patch applies fake diffs from commit messages

#29

Post by em-bee »

yjftsjthsd-h wrote: post #16
phoe-krk wrote: post #12

Earlier quoted context omitted.

If, by "readable by humans", you mean "it would reliably fool humans as well", I'd say it's an ambiguity bug regardless of whether it's "a core feature" or not. A patch format, human-readable or not, should clearly indicate which part is the commit message and which part is an actual diff; it's not the case here.

Alright, allow me to disambiguate in your preferred format. Patch is perfect. Ambiguity is good. There are no better formats for conveying patches. Patch files are readable by humans. Being readble by humans is useful. XML is painful for humans to read and write. JSON is painful for humans to read and write. JSON or XML would actually fix this problem in the format. The patch format could be improved. Formats should…

that's not the preferred format for writing XML, this is:

    
    
      
        Patch is perfect.
        Ambiguity is good.
        There are no better formats for conveying patches.
      
      
        Patch files are readable by humans.
        Being readble by humans is useful.
        XML is painful for humans to read and write.
        JSON is painful for humans to read and write.
        JSON or XML would actually fix this problem in the format.
      
      
        The patch format could be improved.
        Formats should be unambiguous.
        Separating sections is good.

Re: Patch applies fake diffs from commit messages

#30

Post by em-bee »

bombcar wrote: post #20
Avamander wrote: post #14

Earlier quoted context omitted.

This is where I kind-of like the idea of PowerShell, it's just that I dislike almost all other aspects of it and around it.

Same - psh has one good idea and it’s this. The next evolution of shells needs to include it.

can either of you elaborate what you mean? are you talking about support for structured data passing between scripts/programs?
Post reply on HN