Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not converting headings #4

Closed
helgatheviking opened this issue Apr 8, 2014 · 10 comments
Closed

Not converting headings #4

helgatheviking opened this issue Apr 8, 2014 · 10 comments

Comments

@helgatheviking
Copy link

If I have the following as my readme.txt heading:

=== Nav Menu Roles ===
Contributors: helgatheviking
Donate link: https://inspirepay.com/pay/helgatheviking
Tags: menu, menus, nav menu, nav menus
Requires at least: 3.8
Tested up to: 3.8.1
Stable tag: 1.5.0
License: GPLv3

Hide custom menu items based on user roles

When I run wp-readme-to-markdown, I get :

=== Nav Menu Roles ===
**Contributors:** helgatheviking  
**Donate link:** https://inspirepay.com/pay/helgatheviking  
**Tags:** menu, menus, nav menu, nav menus  
**Requires at least:** 3.8  
**Tested up to:** 3.8.1  
**Stable tag:** 1.5.0  
**License:** GPLv3  

Hide custom menu items based on user roles

The heading is not being changed to markdown... which would be # Nav Menu Roles #

@stephenharris
Copy link
Owner

That's really odd - I cannot replicate this (even after copying your readme from wordpress.org). The header should be replaced with

readme = readme.replace( new RegExp("^===([^=]+)===*?[\s ]*?\n","gim"),"#$1#\n");

As you can see it allows spaces after the last ===, but not before the first one, so maybe a space has crept in before === Nav Me...? (I had thought that testing with your actual readme.txt would confirm this, but it seems fine on .org). I could probably add [\s ]*? at the beginning too, to be less strict here.

@helgatheviking
Copy link
Author

Nooo, not the dreaded 'cannot replicate'. :) I even tried using one of your readme.txt (from Event Organizer) files and it still came out the same for me, even though I can see that your readme.md is fine. My readme.md however is not as good-looking.

@helgatheviking
Copy link
Author

I think it has to do with how the line break characters are added when the readme.txt file is parse. Take a look at this fiddle. If you have a new line character, it matches, but if you don't there's no match.

http://jsfiddle.net/helgatheviking/z2k32/

Why the new line characters would be different from your machine to mine is beyond me.

@helgatheviking
Copy link
Author

What about switching the headings regexes with:

        readme = readme.replace( new RegExp("^=([^=]+)=*?[\s ]*?$","gim"),"###$1###");  
        readme = readme.replace( new RegExp("^==([^=]+)==*?[\s ]*?$","mig"),"##$1##");
        readme = readme.replace( new RegExp("^===([^=]+)===*?[\s ]*?$","gim"),"#$1#");

Switching the "\n" to "$" ? And removing the addition of a new line character. If you want new lines, they could be added in the readme.txt. I find adding an empty line after a heading in the readme makes it more readable anyway.

@stephenharris
Copy link
Owner

It passes the unit tests my end, so if it fixes it your end, I'm happy to push the changes.

One thing though: The fact that it seems to be the line-endings is probably (?) because line-endings differ on different operating systems (I'm using linux, and I think you're using a max...?). If it's the use of \n for line endings - this, presumably, breaks the other parts where it's used too? E.g. lines 44, 49, 73.

The last 2 would effect the screenshots section (it tries to guess the image url and use that to display the screenshot). And it does appear that it hasn't parsed correctly on your readme.md. The first instance doesn't happen to effect your readme.md, but it's still a hidden bug.

I think an alternative solution would be to match \r\n (Mac) or \r (Windows) or \n (Linux). Does changing \n to the appropriate line-ending work for you?

@helgatheviking
Copy link
Author

I'm on a Windows machine. Changing to \r does work for me.

I can't really tell if the other places you mentioned are breaking. The contributors, tags, etc seem ok:

=== Nav Menu Roles ===

**Contributors:** helgatheviking  
**Donate link:** https://inspirepay.com/pay/helgatheviking  
**Tags:** menu, menus, nav menu, nav menus  
**Requires at least:** 3.8  
**Tested up to:** 3.8.1  
**Stable tag:** 1.5.0  
**License:** GPLv3  

@stephenharris
Copy link
Owner

Yes the contributor tags should be ok despite this bug. It would only break if you had left empty lines in between each tag:

Contributors: helgatheviking  

Donate link: https://inspirepay.com/pay/helgatheviking  

becomes

**Contributors:** helgatheviking
**
Donate link:** https://inspirepay.com/pay/helgatheviking

The only other thing that breaks is displaying the screenshots. The intended behaviour is that it tries to embed the screenshots (see, for example, Event Organiser - except the last one). There's a lot of guess work involved there (plug-in name, screenshot file type) so it unfortunately doesn't always get it right.

I'll post a patch shortly to deal with the various new-line characters...

@helgatheviking
Copy link
Author

Awesome! I tried copying Event Organizer's screenshot naming convention, and the new commit seems to sort out the screenshots too!

@helgatheviking
Copy link
Author

Sent a little tea your way via the donate link in Event Organizer.

@stephenharris
Copy link
Owner

Great, glad's that working :). And thanks for the tea! 0.7.0 has just been released and contains the fix. (I've also linted the file, and it still passed the unit tests, but if it has broken it your end, just let me know)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants