GNOME Bugzilla – Bug 723815
Avoid extra empty lines in plain quoted text
Last modified: 2014-02-12 19:54:38 UTC
Following up on #723742, there were often extra blank lines appearing at the ends of quotes. The root cause was the the appearance of blank lines at the ends of quotations was unpredictable; sometimes we caught them as part of the quote, sometimes not. After playing around a bit, I realized we could get more predictable behavior by replacing the content of the blockquotes, rather than the blockquotes themselves. This consistently gives us two newlines, one part of the quote, one not. We only need one, so we get rid of the first one. This should make the fix in #723742 unnecessary, since quote tokens should always end up on their own line. But let's keep it around to be safe.
Created attachment 268372 [details] [review] patch
Robert, two questions about your patch (both the same line): * I know this was in the code before you patched it, but I see this in gedit (util-webkit.vala:317 w/ your patch): bq.set_inner_text(@"<unicode character>$i<unicode character>"); These are, as I recall, the Unicode user characters used to delineate blockquotes. They're used up the stack for setting up the indentation. Can you (a) make these characters const somewhere so they're not magic, and (b) make a comment here explaining why they're being inserted? * My other question about this line is why this works -- it looks to me that the counter integer is being inserted, not the quoted text itself. I feel like I'm missing something here. I guess I'm asking for a few more comments to explain what this inner loop is doing. The current comment explains the overall strategy, but looking at the loop's innards I'm not following the steps. Thanks!
Created attachment 268455 [details] [review] patch with more comments Those unicode characters are now the constants QUOTE_START and QUOTE_END. The names aren't quite appropriate -- perhaps QUOTE_TOKEN_START would be better, but that seemed awfully long. I didn't know whether they should be part of the namespace or not. Right now they aren't, since those other constants weren't. This line is indeed taking out the text of the quotes. We are doing this so that when we convert things to text, we don't get the text of any included quotes. Instead, we just get tokens indicating where the quotes go, so that later (line 329) we can reassemble everything with correct quoting. I've expanded the comment before that for loop to try to explain what's happening a bit better.
Thanks! Pushed to master, commit 0e08a8