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

Fix Keep Markup plugin incorrect highlighting #880

Merged
merged 2 commits into from
Feb 6, 2016

Conversation

montogeek
Copy link
Contributor

Hi!

This change fixes #879, it looks like that the Regex replace is messing the correct positions of the markers in the markup, that's why it looked moved one position:
screen shot 2016-02-05 at 22 29 42

After this fix it looks like this:
screen shot 2016-02-06 at 00 05 33

I am not completely sure about this fix because I do not have experience with Prism, probably @Golmote or @LeaVerou can find the real issue.

Thanks!

@LeaVerou
Copy link
Member

LeaVerou commented Feb 6, 2016

Hey, thanks for looking into this! It's a nice little plugin and it's sad that it had that bug.

I’m very surprised this fix works. The new regex won't match newlines in many (if not all) modern systems, since they tend to be \n these days. Perhaps commenting out that line has the same effect?

@zeitgeist87
Copy link
Collaborator

Perhaps commenting out that line has the same effect?

Yes it does. it works perfectly in Firefox and Chrome if you comment out the whole firstWhiteSpaces stuff.

@Golmote
Copy link
Contributor

Golmote commented Feb 6, 2016

I guess I wrote it at a time where the initial line feeds were trimmed in the core.
Not sure how it would behave with the Remove initial line feed plugin...

@zeitgeist87
Copy link
Collaborator

Not sure how it would behave with the Remove initial line feed plugin...

It would probably stop working in combination with any plugin, that trims whitespace or changes the length of the text in any way. Like my proposed whitespace normalizer plugin #847.

@montogeek
Copy link
Contributor Author

Hey! thanks for answer :)

Perhaps commenting out that line has the same effect?

Yes, by removing the replace line or removing the firstWhiteSpaces stuff like @zeitgeist87 said, it completely works.

@LeaVerou
Copy link
Member

LeaVerou commented Feb 6, 2016

Good! Should I close this PR so you can send a new one or do you prefer to update it?

Not sure how it would behave with the Remove initial line feed plugin...

We can make sure to include that plugin before this one.

@montogeek
Copy link
Contributor Author

Hi @LeaVerou I just updated this PR with the latest changes.

LeaVerou added a commit that referenced this pull request Feb 6, 2016
Fix Keep Markup plugin incorrect highlighting
@LeaVerou LeaVerou merged commit 8f79f3b into PrismJS:gh-pages Feb 6, 2016
@LeaVerou
Copy link
Member

LeaVerou commented Feb 6, 2016

Awesome, thanks!

@montogeek
Copy link
Contributor Author

Thanks to you!

@montogeek
Copy link
Contributor Author

Hi @LeaVerou
I noticed that the minified version of the plugin was not updated, it should be updated manually?

I just noticed the Gulpfile task :)

@LeaVerou
Copy link
Member

LeaVerou commented Feb 7, 2016

Ah yeah, you need to run gulp.

@montogeek
Copy link
Contributor Author

So, I should send another PR with the minified version updated to the gh-pages branch?
Or it is automatically updated when the master branch is synced and a new version is released?

I am just trying to understand how is the branch workflow :)

@zeitgeist87
Copy link
Collaborator

So, I should send another PR with the minified version updated to the gh-pages branch?

It is usually a good idea, to include the minified version in your PR. You don't have to create a new PR for keep-markup, because I have updated the minified version for it a few minutes ago.

We periodically run gulp on the gh-pages branch to update all minified files, but there is no fully automatic update process.

@felixsanz
Copy link

@montogeek I used the example from website:

<pre><code class="language-css">
@media <mark>screen</mark> {
    div {
        <mark>text</mark>-decoration: <mark><mark>under</mark>line</mark>;
        back<mark>ground: url</mark>('foo.png');
    }
}</code></pre>

and the bug still happens, its not aligned

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

Successfully merging this pull request may close these issues.

6 participants