Patch 1583 accept and resolve issue2160

Title accept and resolve issue2160
Superseder Nosy List bf
Related Issues
Status accepted Assigned To

Created on 2017-08-13.13:44:51 by bf, last changed 2017-10-22.16:31:57 by ganesh.

File name Status Uploaded Type Edit Remove
accept-issue2160_-wrong-line-numbers-when-appending-empty-line.dpatch bf, 2017-08-13.23:48:27 application/x-darcs-patch
add-comment-to-explain-special-case-in-d_r_diff.dpatch bf, 2017-10-16.06:35:30 application/x-darcs-patch
See mailing list archives for discussion on individual patches.
msg19580 (view) Author: bf Date: 2017-08-13.13:44:51
As explained on the tracker, tests/merging_newlines.sh does and should
now fail, so I also inverted the test.
msg19581 (view) Author: bf Date: 2017-08-13.13:49:04
I am going to wait a few days before screening this in case there are
objections. This is a semantic change and even though I think it is a
bug fix there may be other opinions...
msg19585 (view) Author: bf Date: 2017-08-13.23:48:27
Re-sending the bundle because I messed up the test with one of the patches.
msg19642 (view) Author: bf Date: 2017-09-02.13:00:15
Screening this now since nobody voiced any objections.
msg19741 (view) Author: ganesh Date: 2017-10-12.07:58:54
Looks fine. The special case might perhaps benefit from a comment for 
future readers, but there's plenty of this weirdness already in darcs 
so no big deal.
msg19746 (view) Author: bf Date: 2017-10-12.09:40:50
Ganesh, I fully agree that a comment is in order to explain the special
case. Can you add one? I am pretty much distracted with other stuff ATM.
I really want to get rid of these weirdnesses. I want to look at the
code and say "ah, that's how it works" and be able to move on instead of
"WTF is this doing and why is it so complicated?" (and then spend an
hour trying to simplify it until I realize why exactly it really has to
be that complicated; or else spend another two days to refactor things
until they actually /are/ simpler).
msg19752 (view) Author: bf Date: 2017-10-16.06:35:30
Added a second bundle with a single patch that adds a somewhat longer
comment explaining the issues with lines/unlines and why we need a
special case here.
msg19779 (view) Author: ganesh Date: 2017-10-22.16:31:57
Thanks! I've pushed it to reviewed (from bf-current)
Date User Action Args
2017-08-13 13:44:51bfcreate
2017-08-13 13:49:05bfsetmessages: + msg19581
2017-08-13 23:48:27bfsetfiles: + accept-issue2160_-wrong-line-numbers-when-appending-empty-line.dpatch
messages: + msg19585
2017-08-16 01:06:06bfsetfiles: - accept-issue2160_-wrong-line-numbers-when-appending-empty-line.dpatch
2017-09-02 13:00:16bfsetstatus: needs-screening -> needs-review
messages: + msg19642
2017-10-12 07:58:55ganeshsetstatus: needs-review -> accepted
messages: + msg19741
2017-10-12 09:40:50bfsetmessages: + msg19746
2017-10-16 06:35:30bfsetfiles: + add-comment-to-explain-special-case-in-d_r_diff.dpatch
messages: + msg19752
2017-10-22 16:31:57ganeshsetmessages: + msg19779