Improve apipe a bit #2
Conversation
Still not perfect. It seems one has to Undo twice to have an effect. But it does undo it all at once.
rogpeppe
left a comment
rogpeppe
left a comment
There was a problem hiding this comment.
Thanks so much for having a bash at this. It's been on my TODO list forever, but never quite got up to the top of it... If that "mark", "nomark" trick works, I'll be very happy for a start! Maybe make a PR just for that so it can be trivially merged independently, and put the other, slightly more involved, change into another PR?
| } | ||
| return nil | ||
| if !endsWithNL { | ||
| if _, err := w.Write(nl); err != nil { |
There was a problem hiding this comment.
Hmm, I like the idea but I don't think this is quite right. Consider, for example, a body that consists just of the single character "a". Running apipe sha256sum should result in the body being replaced by sha256("a") but AFAICS this change means it'll be replaced by sha256("a\n").
I think that the only "right" solution here is to cope with the \ No newline at end of file line produced by diff in this case.
There was a problem hiding this comment.
You're right. I attempted to cope with \ No newline at end of file before but gave up. This approach was the one I was able to get done. It suits my usecase well, but you're right it's not a good general solution.
Well, seems like neither of the two fixes is the "right" one. The "mark, nomark" fix seems to have the caveat that you need to click on Undo twice so it actually undos the changes. That too doesn't seem like the "right" solution. Feel free to cherry-pick/modify the commits any way you like :) I guess I can't make any further improvements at the moment. |
2 participants
\nnomark.