emacs-php / php-mode

A powerful and flexible Emacs major mode for editing PHP scripts
GNU General Public License v3.0
583 stars 118 forks source link

Single quote in text in HTML misinterpreted #9

Open e0da opened 12 years ago

e0da commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

in HTML, i.e.

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

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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

etu commented 12 years ago

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

ryuslash commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

@etu

Here is how the formatting appears for me.

.<?php
.echo 'hello';
.
.?>
.
.<a href="/moo">It's true</a>
       .
       .
       .<?php
       .echo 'moo';
.
etu commented 12 years ago

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 commented 12 years ago

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