Diff Popup code highlight not working in PHP file

[churehill]

Diff Popup shows plain code without highlight in PHP file

This may be caused by PHP syntax need <?php to work.

When modified part contains <?php at first line, Diff Popup code highlight works like normal

[r-stein]

This may be caused by PHP syntax need <?php to work.

That's the reason, we just take the source code and use the code highlighter to highlight that specific part. This can obviously be problematic, but usually works well.

[deathaxe]

The diff popup uses mdpopups.get_language_from_view(view) method to determine the syntax to use for highlighting. Highlighting itself is done by mdpopups.

Small hunks which don't represent fully qualified statements are most likely to fail highlighting correctly. I can't see a solution to fix that. In your situation, mdpopups can't know about the <?php which is required for highlighting.

If @facelessuser doesn't have an idea how to handle something like that, we need to live with it.

[facelessuser]

I'll explain how highlight works for mdpopups. It puts the code in a hidden output panel and let's Sublime highlight it, and then uses logic to look at the scope of the text it is pulling out to pull colors from the color scheme file and construct the HTML accordingly.

The other part is as @deathaxe points out, you have to tell it what syntax to highlight with, and the obvious choice is to see what the global syntax for the current view is.

Mdpopups has a mapping of aliases to actual syntax files to use: https://github.com/facelessuser/sublime-markdown-popups/blob/master/st3/mdpopups/st_mapping.py. So basically you reverse this process, using the views syntax, to get a valid alias.

If you copy the snippet you want highlighted into a new view and activate the sources global syntax, you'll see that it doesn't get highlighted because PHP can't just highlight a class method without context of the class, and possibly the <?php.

So snippets of code can be difficult to get fully highlighted without enough of the code's context. The only way to do something like this is to render a full valid snippet and extract a sub portion of it for display. This is what my other plugin ExportHtml does to render any snippet from your fully render code file. But with mdpopups, you are just sending it portions without the other context, so it's ability is limited by what you give it.

I don't know what GitGutter is doing in the picture above, but speaking generally about diffs. If I wanted to show a hunk from the diff (base or current rev), I would most likely need to render the base and current rev in hidden outputs, and use the diff to determine which hunks to grab from those fully rendered files. Only then could you always have full color renders of diff hunks. But there is a price to pay. For each snippet, you have to render the full file, both if you want to show from both previous and current.

Anyways, what I describe would need to be specially built as mdpopups just wants to take snippets. It is possible to get what is desired for a cost, and I don't know if that cost wants to be paid in this plugin. If I've misunderstood what is desired, apologies.

[deathaxe]

Thank you very much for this comprehensive description.

I don't know what GitGutter is doing in the picture above,

GitGutter manages both, a hidden copy of the tree object (committed file) and the content of the current view (working file). It calls git diff --no-index ... to evaluate the changes. The content of the popup above is currently rendered using information parsed from this diff result only as they are available without opening and parsing the two temporary files.

GitGutter could be modified to hold a copy of the tree object's content in memory or a hidden output panel as well. Maybe it could use one mdpopups knows about or manages? This content would be updated only if GitGutter updates the tree object - not too often.

The information of the diff result could then be used to point to the region to use for rendering the correctly highlighted content (in default mode). In diff mode syntax highlighting is disabled anyway.

I already thought about passing the whole tree object's content to mdpopups.syntax_highlight() but extracting the right piece of code from the resulting html seems difficult.

[facelessuser]

Mdpopups uses ExportHTML plugin logic that has been modified and stripped down to do specifically what is needed for mdpopups. This is of course only used when Pygments support is not enabled.

The code is here https://github.com/facelessuser/sublime-markdown-popups/blob/master/st3/mdpopups/st_code_highlight.py.

You can see some functions could take the current output panel selection and return HTML for only that https://github.com/facelessuser/sublime-markdown-popups/blob/master/st3/mdpopups/st_code_highlight.py#L63, but we currently ignore selections. Selections would probably make the most sense as whole lines.

I've obviously de-emphasized Pygments in recent releases, but I still support it. This kind of logic would have to be abstracted to sort of work in Pygments. Most likely to strip out the lines of interest and pass it through normally and if it doesn't highlight, oh well.

So it is possible to add this ability to feed in full text and return only HTML for a subsection, but the way mdpopups works though, it would not retain the full text after render because that is not it's purpose. It is possible that maybe another command could be created where you feed it a view with line numbers, and mdpopups could extract specific lines from it and return HTML for only those lines. The plugin (GitGutter in this case) would manage the output buffers, while mdpopups would use it's logic to extract the text.

mdpopups.syntax_highlight_from_view(view, language, lines, **otheroptions)

[deathaxe]

The idea behind loading the tree object into a hidden view was to avoid loading it repetitively and save some time. As output panels are defined per window, GitGutter would need to create one for each open view to achieve that, which I find a bad approach. One "temporary" panel used as scratch pad is ok. Furthermore HTML creation seems the most expensive task anyway, so adding the new function may not help. From this point of view, it was just enough to be able to pass the desired rendering boundaries with

mdpopups.syntax_highlight(view, source_content, start, end, **popup_kwargs)

source_content would contain the tree object's content and start, end maybe text positions or lines. In my case lines would make more sense as diff popup displays full lines only. Not sure if parts of a line might need to be rendered some day, too.

[facelessuser]

As far as mdpopups is concerned, I can see the benefit of being able to load up a whole view once and grab content from it multiple times. I could see BracketHighlighter for instance using it to render partials via it's bracket hover feature, and it could do it with the current view (though it would be tricky to also emphasize the bracket, so I don't know if I'd actually do that).

GitGutter doesn't have to create a buffer for each view in the window, GitGutter could easily use just one buffer for the window if that was its desire. How GitGutter chooses to use syntax_highlight_from_view isn't really a concern for me, only that it can be applied for its use case.

Thinking about this more, it's possible that I could even provide an option to apply a special class for specified regions to highlight or emphasize certain regions. The plugin dev would manage their buffer(s) and calculate the regions for conversion and for emphasis.

Now, not specifying by lines would be most problematic for Pygments, and in that case, their milage for rendering will greatly vary when specifying portions of a line, but as stated earlier, I am deemphasizing Pygments as I feel the ST highlighting is quite a bit more flexible for use in plugins. Pygments is no longer the default anymore.

[deathaxe]

Now, not specifying by lines would be most problematic for Pygments

Specifying lines fits most needs I think and is just enough.

How GitGutter chooses to use syntax_highlight_from_view isn't really a concern for me, only that it can be applied for its use case.

You want to provide the ability to use the active view as source for highlighting instead of a hidden panel? Sounds reasonable, even though it wouldn't make sense for GitGutter. It rather would need to manage its own hidden output panel which is applied the content of the tree object of the active view to.

What about a scenario with more than one package using this feature? In the end we might have 5 or 10 hidden output panels which all might more or less contain the content of the active view?

As far as mdpopups is concerned, I can see the benefit of being able to load up a whole view once and grab content from it multiple times.

Agree

[facelessuser]

You want to provide the ability to use the active view as source for highlighting instead of a hidden panel?

No, I am thinking about providing the ability to use a view, not specifically active view. My example was only that, a practical example. It could be active, it could not be active, it could be a hidden view, it doesn't matter. They could manage their own view buffer or use an exiting one (without having to create one). That's key; they don't have to create a new view if an open one is already available that suits their needs.

What about a scenario with more than one package using this feature? In the end we might have 5 or 10 hidden output panels which all might more or less contain the content of the active view?

Then they should point at the active view. They don't have to reference hidden output panels, or create a view at all. Also, output panels can be destroyed via destroy_output_panel. They don't have to be left open forever.

With all of that said, I am open to creating a flow that encourages good habits, but also allows people the flexibility to do what's necessary when needed. Yes people can abuse the plugin in special cases, but that can be said about all programming.

I think in most cases, people don't need a persistent panel, and if they do, it would be temporary.

I am open to suggestions as, right now, I am just brain storming and haven't committed to anything.

[facelessuser]

Also, keep in mind, I am approaching this as how can I generally solve issues like this, not specifically how to I solve GitGutter's issue.

[deathaxe]

I am thinking about providing the ability to use a view,

Sorry for my imprecise description of a view. 😄 Of course I had exactly this in mind. Referring to an active view only, wouldn't make sense at all.

... generally solve issues like this, not specifically how to I solve GitGutter's issue.

Absolutely agree. This is what all packages/plugins should do. There is nothing worse than the hell of cross references between packages.

Sounds all well so far. Thank you very much for your time and the very constructive discussion.

[facelessuser]

Cool, I've created an issue linked to this one. I'll probably prototype this feature on a feature branch with some of the ideas here and see how it turns out.

[braver]

Just two cents.

I've never seen GitGutter popups highlight anything so I never even knew it was a feature. Does it ever work?

I think it's totally understandable you cannot highlight a random snippet of code out of context, and getting to that context seems super complicated (and with large files could be a real performance hit). None of the tools I use highlight diffs. I never missed it and might even prefer it to reliably never highlight than highlight once in a blue moon.

---

As an aside:

Does the effort to make this work match the value it provides and is the solution reliable?

I mean, I love the plugins you guys are creating, rely on them every day. But over the years I have seen great plugin concepts go down the drain by trying to do too much without core API's supporting it, and killing performance and/or introducing wonky behaviour in the process. BufferScroll is good example: nice idea, but it's fighting against the API and the application's default behaviour, and it makes the entire editor wonky. Again, I'm not saying it's the case here, just advocating keeping it simple, high performance and predictable.

[deathaxe]

mdpopups provides code highlighting out of the box. Therefore source which is known by mdpopups is highlighted in the diff popup as well as it is automatically put into a <div class=highlight> tag. To do that mdpopups relies on two methods. The legacy one uses pygments and the one we were discussing about here is to use an output_panel of ST to put the snippet into, apply the correct syntax and read out the highlighted code as html. It works quite well for known syntaxes if you set "mdpopups.use_sublime_highlighter": true

The only drawback is what the initial post intends to state. Some syntaxes rely on contexts, which might not be part of the code snippet which is to be displayed in the popup. Therefore highlighting fails.

So we made up our mind how to possibly solve this issue with the existing techniques and strategies being in use already.

There is no concrete plan to implement it at the moment. See mdpopups issues ... it's low priority and more or less a kind of study.

It will not become part of GitGutter if it will create any performance hit or comes with other possible side effects. I already did a lot of studies which have been dismissed without even proposing them due to too many disadvantages. 😉

A absolutely agree with you not to bloat the packages or create any cross references between packages or even fight against the core API. We'll just need to have a short look into the forum to see what it results in after a bigger update. Several such proposals were already declined.

Any of such a bigger change will be proposed as a PR with the chance to discuss or decline it.

[facelessuser]

This is completely doable and I already have some working tests, but I just haven't had the time to clean it up yet and prepare a release.

GitGutter will have to see if they can implement it in a performance friendly way, but there are plenty of applications for the requested feature which are quite reasonable and performance friendly outside of GitGutter. For instance, this feature is going to useful for BracketHighlighter offscreen bracket popups as we will be able to use the existing view's highlighted code to show snippets of code containing offscreen brackets. Performance should be quite reasonable.

If it turns out implementing this in GitGutter is not a good idea due to performance, then that is fine. But, regardless, I think this is a useful discussion as this will help in other plugins, but hopefully GitGutter might get something out of this as well.

[deathaxe]

I'll definitely try to and find it doable, too.