Review 2

[vr8hub]

• [x] Typogrify makes a change that should be accepted.
• [x] As does se clean.
• [x] As does build-images.
• [x] While the cause of commit 16f8d66 is the output of find-mismatched-diacritics, what's actually being done are transcription corrections. This needs to be or at least be mentioned in the commit message, because otherwise it looks like editorial (content) changes are being made without an Editorial tag on the commit message.
• [x] The verse CSS for p > span + br also needs the hymn and song selectors.
• [x] The xml namespace in the CSS should be at the top with the other namespaces (last).
• [x] There's an amour with a language tag.
• [x] The indent on the ch. 5 verse in the scans look more like i3 or 4 rather than i1.
• [x] There is a space missing between the hic! and du mal in ch. 28.
• [x] Also, "fleur du mal" is only in M-W unabridged, so it should be tagged as French.
• [x] As mentioned in the first review, "Woman wailing for her demon lover" is italicized because it's a quote (not because it's emphasis), but it's also quoted, and our house style is that we don't typically do both (quote and italics), so the italics can be dropped.

[bobrk]

The 'se clean .' step made no changes, which I ran after typogrify.

for "amour" there are several cases:

réligion d’amour (I didn't find this anywhere as anything but a french phrase) l’amour (does this count as "amour"?) L’amour, l’amore (amore is a transcription error, according to the scan) L’amour avait passé par là (another French phrase)

How should I handle all of these?

[vr8hub]

Yep, clean and typogrify can make some of the same kind of changes, so since you ran typogrify first (I ran clean first), it also fixed what my clean caught.

My bad on the "amour," I think; I saw one with a tag in one of the later commits, but when I just looked it's not there, so you must have fixed it in a later commit and I didn't notice. The rest are fine.

[bobrk]

Ok, cool, looks like I got everything else on the list...

[vr8hub]

Almost—lint reports an error that should be fixed.

[vr8hub]

Oops; also, the CSS for i3 is incorrect; each increase in the i number should have a corresponding increase in the padding-left, so a two increase in the i equals a two increase in the padding, not one.

[bobrk]

Interesting, I was thinking the iX part was just a label or class, but we have a syntax or rules for that?

So:

i2: 3em i3: 4em i4: 5em

[bobrk]

Current push has them set to i2, looks good to me, but let me know.

[bobrk]

Ok, I figured out the CSS, I think this is what we want.

[vr8hub]

It's good to get in the habit of running lint, clean, and semanticate (at least) every single time before submitting. No matter how minor the change is, you'll be surprised how often one of them catches something.

In this case, lint reports the same error I reported at the start of this round, only now it's worse because two iX classes are being used.

Which, the i1 class isn't needed. What you're seeing in the German verse is called hanging punctuation: it shifts the starting punctuation to just outside the margin so that the actual text is on the margin. Our core.css uses hanging punctuation as well, in some cases. Either way, it is not actual indentation, and so shouldn't have the class.

So, remove the i1 class completely, both from the text and from the CSS, and remove the i2 CSS on the two epub types it's not used on (see lint), and then we should be good.

[bobrk]

Heh, I was running the lint, but I think I got lazy.

Semanticate wanted to change a random 'I' in one of the Italian passages to a roman number, but I rejected that.

For the German one, I know about the hanging punctuation, but it looks to me like it's indented in the scan, unless I'm misreading it. Let me know what you think, and I'll fix the other CSS issues.

[vr8hub]

No, it's not perfectly aligned, but that's not unusual for printing of that age; it's definitely not intentionally indented. (A look at other editions confirms that.)

[bobrk]

It does seem to be too subtle....

[bobrk]

Ok, these changes are made. Thanks for the help!

[vr8hub]

So sorry, Bob; I just noticed that the second blockquote in ch. 5 has two sets of closing quotes and should only have one. (This is one of the ones getting the iX classes, which is the only reason I was looking at it again.)

The line that ends in Miles and miles—’” should be Miles and miles, per the scans. Note that the first blockquote does end in Miles and miles—’” so it might have been a copy and paste error in the transcription.

Also, the commit message for that commit, i.e. c9aefcb2, has the message repeated three times:

    Tweak verse CSS

    Tweak verse css

    Tweak verse CSS

If you would, go ahead and get rid of the extra two versions.

And then we'll be done. I hope. :) (Sorry again; I should have noticed the extra quotes before now.)

[bobrk]

Ugh, I was using GitHub desktop to smash commits. Getting too cocky. Appreciate the attention to detail.