Live data from Hacker News

Patch applies fake diffs from commit messages

samizdat.dev

31–40 of 43 posts

Re: Patch applies fake diffs from commit messages

#31
post #13

Earlier quoted context omitted.

Patch: 1985 SGML: 1986

XML: 1996

what are you suggesting? XML is a simplified form of SGML. an SGML parser can parse XML so it was already possible to write an XML like document before XML was defined.

Re: Patch applies fake diffs from commit messages

#32
post #29

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 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…

What I posted is valid XML. And even prettified, it's a pain to read.

Re: Patch applies fake diffs from commit messages

#33
post #30
post #20

Earlier quoted context omitted.

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?

Yes - https://devblogs.microsoft.com/scripting/working-with-json-d...

Tons of bugs in scripting in Unix come from the fact that data and metadata are interspersed in the same stream (you can mitigate somewhat with stderr vs stdout but hardly anyone does). Examples include things like trying to handle random filenames from * expansions.

It’s a bit more annoying to deal with sometimes, but for actual scripts it’s much more foolproof.

xargs is one of the programs that is designed to work around the original issue.

Re: Patch applies fake diffs from commit messages

#34
post #30
post #20

Earlier quoted context omitted.

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?

Yes, structured data between scripts and programs. No xargs, tee, awk, sed, grep mangling. No "argument list too long" errors.

So many problems are avoided, but at the same time the Windows ecosystem is just so far from providing an properly usable terminal experience. Things are still really not designed to be used from PowerShell.

Re: Patch applies fake diffs from commit messages

#35

Earlier quoted context omitted.

> Seems like this would probably be solved if github returned a patch file formatted like `git show` provides, specifically with the commit message indented? I do see that `git format-patch` doesn't do this indentation though. This would be "solved" if the patch file only included the patch. That's pretty straightforward. The file github provides includes fake email headers for no particular reason. The commit messag…

> fake email headers That's the output you get from `git format-patch --stdout -1 dd28283`. The idea is that it's suitable for emailing to a mailing list for review (hence the subject line beginning with [PATCH], and so on). If you ask for colorized output with `git format-patch --color=always --stdout -1 dd28283` you'll see that `git format-patch` itself knows which bits are the commit message and which bits are the…

> The idea is that it's suitable for emailing to a mailing list for review

It doesn't comply with RFC 5322 ( https://www.rfc-editor.org/rfc/rfc5322#section-2.2 ), which requires that email headers terminate in CRLF.

Re: Patch applies fake diffs from commit messages

#36
post #33
post #30

Earlier quoted context omitted.

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

Yes - https://devblogs.microsoft.com/scripting/working-with-json-d... Tons of bugs in scripting in Unix come from the fact that data and metadata are interspersed in the same stream (you can mitigate somewhat with stderr vs stdout but hardly anyone does). Examples include things like trying to handle random filenames from * expansions. It’s a bit more annoying to deal with sometimes, but for actual scripts it’s much…

i agree.

you may enjoy these recent comments about better terminals:

https://news.ycombinator.com/item?id=47812724

https://news.ycombinator.com/item?id=47811986

https://news.ycombinator.com/item?id=47807195

Re: Patch applies fake diffs from commit messages

#37
post #30

Earlier quoted context omitted.

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

Yes, structured data between scripts and programs. No xargs , tee , awk , sed , grep mangling. No "argument list too long" errors. So many problems are avoided, but at the same time the Windows ecosystem is just so far from providing an properly usable terminal experience. Things are still really not designed to be used from PowerShell.

right, see my response to the sibling comment.

Re: Patch applies fake diffs from commit messages

#38
post #29

Earlier quoted context omitted.

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…

What I posted is valid XML. And even prettified, it's a pain to read.

it was valid but not the way XML is written or read by humans which is what we are discussing. how much of a pain it is to read is a matter of taste. i won't deny that. but XML can be made more readable without fail because it is a structured format. i would not have been ale to reformat a patch text the way i reformatted this XML example. XML is also more powerful. it could handle word based changes, as opposed to patch which can only do line based changes. same goes for JSON. patch could potentially be improved, but i don't see how it could handle word based changes without extra syntax to mark line breaks.

Re: Patch applies fake diffs from commit messages

#39

Earlier quoted context omitted.

> Seems like this would probably be solved if github returned a patch file formatted like `git show` provides, specifically with the commit message indented? I do see that `git format-patch` doesn't do this indentation though. This would be "solved" if the patch file only included the patch. That's pretty straightforward. The file github provides includes fake email headers for no particular reason. The commit messag…

> fake email headers That's the output you get from `git format-patch --stdout -1 dd28283`. The idea is that it's suitable for emailing to a mailing list for review (hence the subject line beginning with [PATCH], and so on). If you ask for colorized output with `git format-patch --color=always --stdout -1 dd28283` you'll see that `git format-patch` itself knows which bits are the commit message and which bits are the…

jolmg points out that if you use a GitHub URL ending in .diff instead of .patch, you get something much more suitable to feed mechanically into `patch`. (And probably not so exploitable.)

Therefore I retract my claim that this is even a "misuse" of `git format-patch` by GitHub. Seems like GitHub provides both a git-am-able endpoint and a (less exploitable) patch-able endpoint, and the issue is just that OP chose the less suitable one of those two endpoints.

Re: Patch applies fake diffs from commit messages

#40
post #23

But what problem does this actually introduce? If you are applying a patch you must already trust the source anyways and this isn’t harder to spot than a rogue file anywhere else in the patch as it looks the same.

Field confusion. If you manually reviewed the diff in GitHub but did not pay attention to the commit message (which GitHub already collapses when long or may start doing any day they want), you are screwed.

Imagine the diff only has

  if (someIntParamThatShouldHaveBeenUInt 
Would you care who wrote it?
Post reply on HN