wf49670 / ppgen

Post-processing generator for DP
6 stars 4 forks source link

RGB color codes sometimes mistaken as ppgen internal links #73

Closed davem2 closed 9 years ago

davem2 commented 9 years ago

Hi Walt,

I came across an issue in the preprocessing of PPer supplied internal links (e.g. #linkname:id#). If a .de statement contains two or more RGB color codes, then part of the statement is mistakenly identified as a internal link.

Here's an example of the problem:

ppgen input: .de div.test { background-color: #eeeeff; border: dashed 1px #aaaaaa; }

Generates the HTML div.test { background-color: ⑲eeeeff; border: dashed 1px ⑲aaaaaa; }

I've solved the issue on my end by ignoring .de lines when flagging internal links during preprocess(). A better solution might be to narrow the regex (don't allow a space before closing #). This bug could also potentially affect .li as well or anywhere where RGB color codes are allowed.

-David

wf49670 commented 9 years ago

Thanks, David. Good catch.

I'll look at fixes, and get something into the development version soon.

In many places we do exempt .li from such operations, but we don't seem to do that for the internal links; I'll take care of that, too. Also, I see that for that form of link we allow zero-length elements, e.g, #:x# or #x:# or even #:#. Clearly those would cause validation errors, but I'm not sure I like allowing them through so I'll have to think about that case, too.

A .de fix is not completely straightforward, though it is doable, because of continuation lines.

I like your idea about not allowing a space before the closing #, but I think that can be extended to not allowing a space at all. It is an ID value, and they're restricted to [A-Za-z0-9-:.] so it may be enough to incorporate that into the regex that looks for internal links instead of using .? as it does today. That is, instead of #(.?:.?)# we could look for #(.+?:[A-Za-z][A-Za-z0-9-:.]?)#

Anyway, I'll take a look at it and get something pushed into development.

Again, thanks, Walt

On 2/5/2015 4:40 PM, davem2 wrote:

Hi Walt,

I came across an issue in the preprocessing of PPer supplied internal links (e.g. #linkname:id#). If a .de statement contains two or more RGB color codes, then part of the statement is mistakenly identified as a internal link.

Here's an example of the problem:

ppgen input: |.de div.test { background-color: #eeeeff; border: dashed 1px #aaaaaa; }|

Generates the HTML |div.test { background-color: ⑲eeeeff; border: dashed 1px ⑲aaaaaa; }|

I've solved the issue on my end by ignoring .de lines when flagging internal links during preprocess(). A better solution might be to narrow the regex (don't allow a space before closing #). This bug could also potentially affect .li as well or anywhere where RGB color codes are allowed.

-David

— Reply to this email directly or view it on GitHub https://github.com/wf49670/ppgen/issues/73.

Brian51 commented 9 years ago

edited out

wf49670 commented 9 years ago

@Brian: I'm not sure what you mean by "will validate", Brian. Can you clarify that, please?

@David: Another thing you can do, today, is use : instead of : in the .de that has the problem. I'm also going to allow # which ppgen doesn't recognize today.

wf49670 commented 9 years ago

I am concerned about one aspect of using #(.+?:[A-Za-z][A-Za-z0-9-_:.]*?)# for the serach term: it will make it harder (if not impossible) to validate the ID part of the value and warn the user about invalid ones. Instead, ppgen would not recognize the #...:...# string at all, and it would simply end up in the output file waiting for the user to discover it while examining the output.

The approach of skipping .li blocks (as we should have been) and exempting .de statements is cleaner, in that regard. And if it completely resolves the problem it may be the better way to go.

wf49670 commented 9 years ago

Fixed in 3.46g by exempting .de statements and .li blocks from the page link processing.

(I did find one regression test case where the PPer used a page link of the form #text:ID-value# within a .li block, so I also clarified the .li documentation.)

Thanks again, David, for reporting the problem.