<html><head></head><body>Hey Jack,<br>
<br>
Yes, I agree with you and usually try to keep commits logically distinct. This was one of those that kept spidering out. Initially, I was trying to fix the default CSS we provide-- specifically in the case of tenseChange, and found other CSS that was wrong... Ok, this commit can expand to be a default CSS logical change... And when looking at the default CSS in the other filters, I found initialization errors which might prevent the CSS from being correct... and then the <lg> formatting which wasn't exactly a CSS change but still a formatting change... It all should have been split out, but I didn't foresee it growing as large as it did. <br>
<br>
Regarding the <div></div> for lg, an empty div usually is rendered with no height but does induce a line break, by default. I didn't want to force additional vertical whitespace, in the event there was already a new line. Specifically in this case, this pushes a <div></div> into the preverse material if the verse starts with lg or l. This cleans up printing a verse number and then immediately a new line after the verse number, which doesn't look nice. <br>
<br>
My apologies again for the large commit. I agree, small commits are the goal,<br>
<br>
Troy<br><br><div class="gmail_quote">On December 20, 2017 5:42:17 PM MST, Jaak Ristioja <jaak@ristioja.ee> wrote:<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">Hello!<br /><br />The last commit to SVN trunk (3547) on December 10 was a large one. The<br />commit message stated 7 different changes, one of which was "Fixed a ton<br />of initialization bugs in filters". Really nice bug finds! But it was a<br />terrible commit to try to understand. While trying to keep Sword++ in<br />sync with Sword SVN trunk, I ended up splitting that one commit into<br />about 20 different logical changes. Because I needed to grasp all the<br />separate changes it introduced, I think it was the most sensible thing<br />to do, given my limited intellectual abilities.<br /><br />Doing so, I think I also found a bug accidentally introduced by the<br />commit into OSISXHTML::MyUserData::outputNewline(), which instead of<br />appending a newline to the preverse, appends "<div></div>", causing<br />tests to fail. You might want to take a look, because it seems to have<br />made it into the sword-1-8-0 tag.<br /><br />Please try to avoid such mega-commits, if possible. Such are extremely<br />difficult to review, and scare people away. Try to "Commit early, commit<br />often", splitting your work into small logical changes which are easier<br />to reason about. Logical commits in version control history also help<br />review of source code at a much later time, e.g. when developers dig<br />deep into the history to find some answers, having to reason their way<br />through such huge monoliths is usually very counter-productive.<br /><br />I don't know how good SVN clients are nowadays, but with git, if one has<br />a ton of changes, one can selectively stage (for committing) a subset of<br />them using `git add`, `git add -p` or similar, and so effectively split<br />their changes into logical commits before pushing these to the central<br />repository.<br /><br /><br />Thank you and God bless!<br />J<br /><br /><hr /><br />sword-devel mailing list: sword-devel@crosswire.org<br /><a href="http://www.crosswire.org/mailman/listinfo/sword-devel">http://www.crosswire.org/mailman/listinfo/sword-devel</a><br />Instructions to unsubscribe/change your settings at above page<br /></pre></blockquote></div><br>
-- <br>
Sent from my Android device with K-9 Mail. Please excuse my brevity.</body></html>