Patch 1583: accept and resolve issue2160 - Darcs bug tracker

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

Files
File name	Status	Uploaded	Type	Edit	Remove
accept-issue2160_-wrong-line-numbers-when-appending-empty-line.dpatch		bfrk, 2017-08-13.23:48:27	application/x-darcs-patch
add-comment-to-explain-special-case-in-d_r_diff.dpatch		bfrk, 2017-10-16.06:35:30	application/x-darcs-patch

See mailing list archives for discussion on individual patches.

Messages
msg19580 (view)	Author: bfrk	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. Attachments accept-issue2160_-wrong-line-numbers-when-appending-empty-line.dpatch
msg19581 (view)	Author: bfrk	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: bfrk	Date: 2017-08-13.23:48:27
Re-sending the bundle because I messed up the test with one of the patches. Attachments accept-issue2160_-wrong-line-numbers-when-appending-empty-line.dpatch
msg19642 (view)	Author: bfrk	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: bfrk	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: bfrk	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. Attachments add-comment-to-explain-special-case-in-d_r_diff.dpatch
msg19779 (view)	Author: ganesh	Date: 2017-10-22.16:31:57
Thanks! I've pushed it to reviewed (from bf-current)

History
Date	User	Action	Args
2017-08-13 13:44:51	bfrk	create
2017-08-13 13:49:05	bfrk	set	messages: + msg19581
2017-08-13 23:48:27	bfrk	set	files: + accept-issue2160_-wrong-line-numbers-when-appending-empty-line.dpatch messages: + msg19585
2017-08-16 01:06:06	bfrk	set	files: - accept-issue2160_-wrong-line-numbers-when-appending-empty-line.dpatch
2017-09-02 13:00:16	bfrk	set	status: needs-screening -> needs-review messages: + msg19642
2017-10-12 07:58:55	ganesh	set	status: needs-review -> accepted messages: + msg19741
2017-10-12 09:40:50	bfrk	set	messages: + msg19746
2017-10-16 06:35:30	bfrk	set	files: + add-comment-to-explain-special-case-in-d_r_diff.dpatch messages: + msg19752
2017-10-22 16:31:57	ganesh	set	messages: + msg19779