-
-
Notifications
You must be signed in to change notification settings - Fork 603
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
Syntax highlighting breaks if the code starts with a partial comment #214
Comments
Whats your idea to work arround it? Just showing more lines of code? |
Nah, that won't ever be guaranteed. |
@denis-sokolov That's the solution I tried once and it was, as far as I remember, consistently reliable -- memory fails me as to why I ended up not moving forward with it, unfortunately. It ended up looking something like: /** (comment start added by whoops)
* @return bool Some result
*/
public function theFooing($muchFoo = true)
{ Though indentation can also be corrected if that's deemed necessary. |
Sounds great |
Great, thanks for your input, @filp. |
A fix for this should also take care that #205 won't get unfixable. |
@staabm Given the fairly simple process for fixing this issue, maybe it can be done in JS and applied to the source file at the time syntax highlighting is applied. Would also mean this transformation would only need to be done on parts of the code that the user is actually look at (I don't have the numbers to back it up, but I assume in the majority of cases users don't actually navigate to frames other than the first). |
I navigate frames a lot. Not that it will hinder a fix significantly. |
What if the missing comment start was the reason the code was actually failing? Then you're just hiding the error. |
That is a good observation, @ConnorVG! |
If the error is the comment, the beginning of the snippet won't be the comment itself since it takes a few lines before, no ? |
@Anahkiasen not really... Missing |
I had an idea that I thought might work. After This is my initial attempt at that idea: $firstLine = trim(reset($range));
if ($firstLine) {
$firstCharIsAsterisk = $firstLine[0] === '*';
$giveup = 10; // give up after 10 tries to find the comment head
$tryCtr = 0;
while ($firstCharIsAsterisk !== false && $tryCtr < $giveup) {
$range = $frame->getFileLines($line - 8 - $tryCtr, 10 + $tryCtr);
$firstLine = trim(reset($range));
$firstCharIsAsterisk = $firstLine[0] === '*';
$tryCtr++;
}
} The problem is that intermediary lines of multiline comments don't have to start with an asterisk. My idea worked with this comment: /*
* comment 1
* comment 2
* comment 3
* comment 4
* comment 5
* comment 6
* comment 7
*/
$t = 1/0; // exception here I got a larger code sample window that included the error line all the way up to the comment head. You could also just use that code to verify that there IS a comment head somewhere in there, then go with the original range but append the opening comment as filp mentioned. This comment, however, broke my idea: /*
comment 1
comment 2
comment 3
comment 4
comment 5
comment 6
*/
$t = 1/0; // exception here Most people use the asterisk all the way down, but it isn't necessary. Without that as a hint, there's really no way I can think of to evaluate whether a given line, in isolation, is a comment or not. Thought I would share my efforts in case this helps someone else think of a solution, or at least see what does not work. There may be a variation of this that does work, for example, you could start at the bottom of the range and look for the comment tail (*/), then you know that there ought to be a head somewhere above it. |
I'd say we need to match on |
Closing due to inactivity. If anyone wants to work on this, feel free to speak up. |
It's not just comments that we need to look for: print('Strings can
span multiple lines.
The following line:
/*
has no effect
because comments
cannot be started
inside strings.'); To have any real reliability we need to deal with the code from the beginning of the file to the end of the code we want to display. We could do one of the following with the said code:
If option 1 or 2, does anyone have any recommendation of PHP-based server-side syntax highlighters? If option 2 and we want to worry about the server-side and client-side syntax highlighters behaving exactly the same, perhaps we could convert |
We could be naughty: syntax highlight the file then extract the portion we need. rubs hands together manically |
@filp @denis-sokolov how about #436 ? |
@acidjazz @denis-sokolov This still sometimes not work. See #588 and googlearchive/code-prettify#553 for details. |
We should be able to detect such situations and workaround them.
The text was updated successfully, but these errors were encountered: