facelessuser / sublime-markdown-popups

Markdown popup dependency for Sublime
https://facelessuser.github.io/sublime-markdown-popups/
Other
112 stars 13 forks source link

Syntax spillage across code blocks #98

Closed rchl closed 4 years ago

rchl commented 4 years ago

Sublime 4081

This code shows a bug where the syntax of the first code block (sh) seems to be used for the second block (json) also.

import mdpopups;mdpopups.show_popup(view, "```sh\nnode \"/file.js\" --stdio\n```\n - languages\n```json\n[\n  {\n    \"document_selector\": \"source.json\", \n    \"feature_selector\": \"source.json\", \n    \"language_id\": \"json\"\n  }\n]\n```")

There is currently no mapping for JSON syntax in ST4 which might have something to do with that. I'm fixing that in #97.

Here is an example with just the second block:

import mdpopups;mdpopups.show_popup(view, "```json\n[\n  {\n    \"document_selector\": \"source.json\", \n    \"feature_selector\": \"source.json\", \n    \"language_id\": \"json\"\n  }\n]\n```")

Actually, after trying those examples in order, I've noticed that the second one just uses the syntax of what was used previously, even across mdpopups invocations. So there is some global state somewhere being reused.

facelessuser commented 4 years ago
Sorry, I didn't read the open post as closely as I should, see second post. I'm pretty sure what you think is happening is not happening. This logic is controlled by Python Markdown which is heavily tested. The language comes from the language name you provide. We have altered the highlighter for Sublime so that it can extract the colors from a view though, So, we extract settings from the view you provide for things like color schemes, but we create a hidden view where we load the code and set the syntax, then extract the colors. https://github.com/facelessuser/sublime-markdown-popups/blob/master/st3/mdpopups/st_code_highlight.py#L235 I don't think we ever explicitly destroy that hidden view buffer though, so maybe, if something goes wrong with setting syntax, maybe we can use the syntax already there? I'd have to look at the logic and see if there is a way we could screw up like that. Keep in mind, I'm not using your pull for `JSON` yet, but if I run these two commands back to back, I get what I expect: ```py import mdpopups;mdpopups.show_popup(view, "```sh\nnode \"/file.js\" --stdio\n```\n - languages\n```json\n[\n {\n \"document_selector\": \"source.json\", \n \"feature_selector\": \"source.json\", \n \"language_id\": \"json\"\n }\n]\n```") ``` ```py Python 3.3>>> import mdpopups;mdpopups.show_popup(view, "```python\nimport test\n```") ``` You have an issue trying to highlight `JSON` right after. I'll make the change locally and verify.
facelessuser commented 4 years ago

I think I misunderstood your complaint. I now see in your first example you have two fences in the same Markdown.

Here's the issue. This is a Markdown parsing issue you are running into, but it is also, currently, an understood and accepted behavior. Generally in Python Markdown, blocks are separated with an empty new line.

The following wouldn't give you two paragraphs:

paragraph
paragraph

This would:

paragraph

paragraph

This applies to other blocs as well.

Python 3.3>>> markdown.markdown('test\n- list\ntest')
'<p>test\n- list\ntest</p>'

So, now I think you can see why this might not work.

My SuperFences documentation goes into syntax requirements with this in mind: https://facelessuser.github.io/pymdown-extensions/extensions/superfences/#nested-fence-format.

Now, yes, some Markdown parsers will handle this and some won't.

facelessuser commented 4 years ago

In short, changing your content as follows should work:

Python 3.3>>> import mdpopups;mdpopups.show_popup(view, "```sh\nnode \"/file.js\" --stdio\n```\n\n - languages\n```json\n\n[\n  {\n    \"document_selector\": \"source.json\", \n    \"feature_selector\": \"source.json\", \n    \"language_id\": \"json\"\n  }\n]\n```")
Screen Shot 2020-08-15 at 8 13 10 AM
facelessuser commented 4 years ago

@gir-bot remove S: triage @gir-bot add T: support, S: wontfix

rchl commented 4 years ago

No, it doesn't work. The content of the second block should not be highlighted because there is no correct syntax mapped for json (without https://github.com/facelessuser/sublime-markdown-popups/pull/97).

See, I'm getting this: Screenshot 2020-08-15 at 16 26 58

which is the same that I would get when using sh language for the second block:

import mdpopups;mdpopups.show_popup(view, "```sh\nnode \"/file.js\" --stdio\n```\n\n - languages\n```sh\n\n[\n  {\n    \"document_selector\": \"source.json\", \n    \"feature_selector\": \"source.json\", \n    \"language_id\": \"json\"\n  }\n]\n```")

If I change the first block to js (and leave the second one with json), notice how it also affects the highlighting of the second block (braces highlighted differently): Screenshot 2020-08-15 at 16 29 54

rchl commented 4 years ago

Since you've explained how it works, I presume that when setting the syntax in the temporary view for that second block, that fails and so the syntax is left at previously used one which is sh.

facelessuser commented 4 years ago

I missed a new line before the second block:

import mdpopups;mdpopups.show_popup(view, "```sh\nnode \"/file.js\" --stdio\n```\n\n - languages\n\n```json\n\n[\n  {\n    \"document_selector\": \"source.json\", \n    \"feature_selector\": \"source.json\", \n    \"language_id\": \"json\"\n  }\n]\n```")
facelessuser commented 4 years ago

Is this without the new JSON syntax rule you are adding or with? I'm a bit confused.

facelessuser commented 4 years ago

Let me run some tests to determine what syntax gets assigned in these cases.

rchl commented 4 years ago

Without. That's probably what is contributing to the issue. I would think the issue is obvious when running the examples or seeing the screenshots. I think you must be on the go or something if you can't see it. :) EDIT: Your snippet with extra newline fails in the same way.

facelessuser commented 4 years ago

Oh, shoot, is there no longer a Plain text syntax in ST4?

facelessuser commented 4 years ago

So, if a syntax fails, we look for and assign Plain text, but I imagine it can't verify the resourse for Plain text as I cannot find one in Sublime anymore...

facelessuser commented 4 years ago

Oh, it is under Text now.

facelessuser commented 4 years ago

Or maybe it has always been there...my fallback Packages/Plain text looks wrong as it doesn't specify the package...

rchl commented 4 years ago

So moved similary to JSON syntax. I haven't noticed that myself. :)

I wonder if there is more.

EDIT: No, it seems to be the same in ST3 and ST4: Packages/Text/Plain text.tmLanguage

facelessuser commented 4 years ago

Yeah, I was just an idiot, and apparently no one noticed until now.

facelessuser commented 4 years ago

I got it working now..

facelessuser commented 4 years ago

@gir-bot remove S: wontfix, T: support @gir-bot add T: bug