RegexOptions.Multiline expression changes about markdownsharp

[GoogleCodeExporter]

hmm. The nature of some of these changes, I am not sure they are correct. Did 
you
verify the unit tests still pass (no new failures were created)?

For example, you changed

(?=\n+|\Z)

to 

$

The above is a positive lookahead, whereas the below is a simple "match end of 
line".
At the very least, I think it would need to be:

(?=$)

And even then, deviating too far from the regexes in the markdown.pl v1.0.1 and
markdown.pl v1.0.2b8 is IMHO not a good idea -- at least not without a VERY good
reason. (See my DeTab() changes for one time that can happen..)

Original comment by [email protected] on 28 Dec 2009 at 6:49

[GoogleCodeExporter]

get latest -- I added a crc-16 to the output HTML file generation in
GenerateTestOutput("mdtest-1.1"). Confirm that no *.xxxx.actual.html files 
change --
the xxxx is the crc-16 of that file -- between the version you get from source
control, and the version with your proposed regex changes.

Original comment by [email protected] on 28 Dec 2009 at 8:52

[GoogleCodeExporter]

can you send a new patch of *just* the changes you are proposing here?

The other patch has a bunch of unrelated changes..

Original comment by [email protected] on 29 Dec 2009 at 2:47

[GoogleCodeExporter]

Hi Jeff,

This change isn't as dramatic as it was before the compiled flags were added, 
but I 
think it cleans up some of the Regex logic and uses some built in processors 
for the 
RegexOptions.Multiline instead of doing the start and end lines manually.

Nick

Original comment by nberardi on 29 Dec 2009 at 3:01

Attachments:

• markdown-multiline-patch.patch

[GoogleCodeExporter]

Thanks. I grabbed the patch, applied it, and tested.

Decline for two reasons:

1) that block is about to change majorly when I pull across the new HTML block
detection in markdown.pl 1.0.2b8 (fixes the two test failures in 
/mdtest-1.1-alt)

2) I'm not a big fan of changing the regexes from the original Perl versions 
unless
there is a really COMPELLING reason to do so. I don't see any performance 
benefit in
the benchmarks, or additional correctness from making these changes.. so 
they're just
change for the sake of change in an area where it's a bad idea to have change --
because we want to be able to fully sync with the Perl version whenever we can.

Original comment by [email protected] on 29 Dec 2009 at 4:14

• Changed state: WontFix

[GoogleCodeExporter]

Agreed.  There seemed to be a performance gain, but it has been mitigated by 
changes 
since I submitted this.  

Also I was unaware that markdown (perl version) was still being actively 
developed.  So  
(2) makes alot of sense to me now.  I will keep that in mind for any future 
patches I 
submit.

Original comment by nberardi on 29 Dec 2009 at 4:39