drmuey / p5-Text-Extract-MaketextCallPhrases

Extract phrases from maketext–call–looking text
0 stars 6 forks source link

Multiline maketext calls report incorrect line & offset #10

Open ColMelvin opened 7 years ago

ColMelvin commented 7 years ago

If the maketext call spans multiple lines the line or offset will be incorrect, specifically:

This issue causes lt edit to fail, subtly, since it cannot find the location to edit.

drmuey commented 7 years ago

Thanks! I'll have to ponder over this a bit.

I do see value in having the correct data so that that tool can make it easier to edit the WS violation.

We purposefully discourage phrases** that contain newlines and I almost think its ok that doing so makes you have to fix it so it’ll stick for next time.

** See “Rationale” at http://search.cpan.org/perldoc?Locale%3A%3AMaketext%3A%3AUtils%3A%3APhrase%3A%3ANorm%3A%3AWhiteSpace for more details.

ColMelvin commented 7 years ago

The lt edit feature already disallows editing any phrases with newlines in them; it skips them without queuing. As such, this change does not make it easier to edit them.

I made the fix for the general case so this public module would be consistent in its output, and therefore, more reliable to all consumers.

That said, the first case, a newline between the phrase and the maketext call (but not in the phrase itself), quietly hits us with lt edit. It discards changes we may have made dozens of phrases earlier and it often isn't caught until after the string review. That means we have to re-change them from scratch, and our memories don't always yield the same high-quality, docs approved message.

drmuey commented 7 years ago

cool thanks