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

Single quote in text in HTML misinterpreted #9

Open
e0da opened this issue Nov 2, 2011 · 20 comments
Open

Single quote in text in HTML misinterpreted #9

e0da opened this issue Nov 2, 2011 · 20 comments
Labels

Comments

@e0da
Copy link

e0da commented Nov 2, 2011

If you have a single quote in HTML text, like <a href="http://example.com">You're right!</a>, the single quote is interpreted as beginning a quoted string.

@ejmr
Copy link
Collaborator

ejmr commented Nov 2, 2011

Do you mean if you have that text assigned as a string in PHP code itself, or if you have PHP and HTML in the same file together?

@e0da
Copy link
Author

e0da commented Nov 2, 2011

in HTML, i.e.

<?php echo('ouch'); ?>
<a href="somewhere">We'll go there</a>

@ejmr
Copy link
Collaborator

ejmr commented Nov 2, 2011

Ah gotcha. Thanks for the clarification.

The code that I started working from is not really meant to handle PHP + HTML on its own. It has support for detecting and trying to work with things like 'mumamo' that support multiple major modes in a single buffer; but this version of php-mode does not try to do anything to plain HTML itself. So I am not surprised that this problem comes up.

What do you think a good solution would be? For people who need all of the highlighting of plain HTML plus that of PHP, I think the existing multiple major mode solutions are good enough. But personally I rarely work with PHP mixed with HTML. In that scenario, do you think it would be good enough if php-mode simply applies a blanket color to all of the plain HTML, if only to help avoid formatting issues like this one?

@e0da
Copy link
Author

e0da commented Nov 2, 2011

Hmm. I'm not sure. Maybe this bug isn't that significant? I mean, while you can mix HTML and PHP, you really shouldn't be doing it anyway. I only noticed because I was reading somebody else's icky code in Emacs.

Maybe the blanket color for all HTML is the way to go? And there could be a partially supported option for highlighting everything?

@ejmr
Copy link
Collaborator

ejmr commented Nov 2, 2011

Personally i feel the same way, that you really shouldn't be mixing PHP and HTML together. But like you hint at there's a lot of code out there like that. So it would probably be useful to add at least an option to provide some blanket highlighting for all of the HTML for people who just want to focus on the PHP without seeing a lot of weird formatting.

Thanks for bringing this to my attention.

@etu
Copy link
Contributor

etu commented Jun 11, 2012

bump? I would be very interested in getting this fixed... I don't know elisp myself... but been using the old php-mode 1.5 for years and years, and well... starting to get out of date...

And I think this is the only bug that will affect me that I noticed that will affect my work... at work, we have much bad code...

And btw, thanks for the fork, and the upgrades and stuff... :)

@ejmr
Copy link
Collaborator

ejmr commented Jun 11, 2012

I will take another swing at this. The idea that came to mind was to look for

  1. Everything from the start of the buffer to <?
  2. Everything between ?> and <? characters.
  3. And everything from ?> to the end of the buffer.

and blanket wrap all of that in font-lock-constant-face or something. The regular expressions are not working though, and this being Elisp I assume that I forgot to add twenty-six consecutive backslashes somewhere. I also suspect this approach would bring chaos on the highlighting of anyone doing something like using PHP to generate PHP (I know somebody is doing it), but I'll just sweep that under the rug for now.

I feel like applying some blanket color to all of the characters outside of the pre-processor tags would be useful enough. And I also feel like I am missing something really obvious as far as getting that to work. But I'll have at it tonight and the next few days and try to get something working.

@ejmr
Copy link
Collaborator

ejmr commented Jun 21, 2012

I still have not found a satisfactory solution to this problem but I just wanted to bump this thread as an indication that I have not forgotten about it.

@etu
Copy link
Contributor

etu commented Jun 25, 2012

Well, If you find a solution for it... I get email about if if you reply in this ticket... As you said yourself -- you shouldn't mix text into your php. :)

Thanks for your time anyways :)

@ryuslash
Copy link
Contributor

A simple workaround is, of course, to simply use html entities such as &#39;, which is what I do whenever I see a '

@etu
Copy link
Contributor

etu commented Jun 25, 2012

Tell that to my 500MB of old code... made by non-programmers ;)

@ryuslash
Copy link
Contributor

Tell that to my 500MB of old code... made by non-programmers ;)

Ouch, you're right, it's not a very good work-around in that case...

@ejmr
Copy link
Collaborator

ejmr commented Jun 26, 2012

Yeah I would love to just take the attitude of, "There is no reason to address this because you're doing it wrong anyways." But the reality is there are probably more programmers maintaining bad existing code than writing new stuff.

@ejmr
Copy link
Collaborator

ejmr commented Aug 23, 2012

Ok, I am looking at this again today. I am sorry I do not have progress to report; I just want to vent. This seemingly simple task is frustrating the Hell out of me because I feel like there is some easy solution representable in Elisp regular expressions that I just do not understand. Also, the syntax for regular expressions in Elisp is God-awful horrid and should be abolished completely in favor of the rx macro.

If anyone watching this repo---and I know you're out there---are skilled with regular expressions in Emacs then please consider taking a stab at fixing this. I would greatly be in your debt, as I know there are many programmers out there who unfortunately have to deal with bad codebases that mix PHP and HTML, and this big affects them.

@rev22
Copy link
Contributor

rev22 commented Aug 24, 2012

I cherry-picked something from my fork that fixes the problem reported.

It unsets font-lock-syntactic-keywords and changes the syntax table and php-font-lock-keywords-1

https://github.com/rev22/php-mode/compare/fix-single-quote

I am not pushing this fix yet as it may quite possibly break other things, more testing would be needed.

@ejmr
Copy link
Collaborator

ejmr commented Aug 24, 2012

Thank you rev22!

I cherry-picked your commit and merged it into my master branch. I would like to hear the opinions from others before closing this issue, but in my opinion this is certainly an improvement. I tested it out by opening random PHP files in large code-bases and nothing looked wrong. Anyone please let me know if these causes a font-lock problem that I need to revert.

@etu
Copy link
Contributor

etu commented Aug 24, 2012

Woho, much better (but not perfect)! I will show you here:

.<?php
.echo 'hello';
.
.?>
.
.<a href="/moo">It's true</a>
    .
    .
.<?php
    .echo 'moo';
    .

I added a dot at end of every line to show where my key think it's correctly indented. Which is quite... intresting... (Might be my smart-tabs or something! Just tell me if this doesn't happen you!)

@ejmr
Copy link
Collaborator

ejmr commented Aug 24, 2012

@etu

Here is how the formatting appears for me.

.<?php
.echo 'hello';
.
.?>
.
.<a href="/moo">It's true</a>
       .
       .
       .<?php
       .echo 'moo';
.

@etu
Copy link
Contributor

etu commented Aug 26, 2012

That's not quite right either in my opinion... But that's a new ticket I think. I must say that this one is solved :)

@ejmr
Copy link
Collaborator

ejmr commented Sep 21, 2012

The fix for issue #21 re-introduces this bug so I am re-opening this ticket again.

@ejmr ejmr reopened this Sep 21, 2012
haxney added a commit to haxney/php-mode that referenced this issue Jan 28, 2013
This allows regression testing without needing manual inspection.
haxney added a commit to haxney/php-mode that referenced this issue Jan 28, 2013
This allows regression testing without needing manual inspection.
ejmr pushed a commit that referenced this issue Jan 28, 2013
* haxney/ert-tests:
  Mark tests 9, 14, and 19 as expected failures.
  ERT tests for remaining example files.
  Require `php-mode` in `php-mode-test.el`, duh.
  ERT test for #19
  ERT tests for issues 14, 16, and 18
  ERT test for issue #8.
  Create ERT-based test for issue #9.
jorissteyn added a commit to jorissteyn/php-mode that referenced this issue Aug 31, 2014
Issue emacs-php#9 is not fixed (and expected to fail) but because of the
different highlighting of unmatched string delimiters in the cc-mode
branch the test passed. This commit makes the assertion more specific so
it fails again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants