marshallward / vim-restructuredtext

Syntax file for reStructuredText on Vim.
26 stars 12 forks source link

interpreted text and codeblock nested within admonition block not highlighted #54

Closed idnsunset closed 3 years ago

idnsunset commented 4 years ago

Interpreted text enclosed in quotes and sphinx code-block are not highlighted when nested within an admonition block.

2020-04-30-181513_437x258_scrot

By the way, when placed in a table cell, the code-block is not highlighted either (interpreted text works). I am not sure whether it will introduce additional performance problem when matching code blocks in table.

marshallward commented 4 years ago

Yes, I can reproduce. You are right, it sounds like admonition directives like note need to be handed as formatted text.

https://docutils.sourceforge.io/docs/ref/rst/directives.html#admonitions

It probably won't affect performance, if we can exclude it somehow from the standard directive formatting rules.

madjxatw commented 4 years ago

I've made a fix (https://github.com/madjxatw/vim-restructuredtext/commit/c267fece6febcd94673d353294f570c4d920487f) for this issue. If you think it's OK, i could send you a PR.

marshallward commented 4 years ago

@madjxatw I had thought the issue was that admonition blocks needed to be highlighted as regular reST-formatted text.

~Your patch seems to correctly add highlights to the reference inside the note block, although it does not restore highlighting to the code block. It also would (afaik) apply this change to all such directive blocks, outside of specific exceptions like .. code::.~

I haven't yet read carefully what exactly an admonition block is, but I think what is wanted here is to "disable" the standard directive block highlighting and just handle the entire block as regular reST formatting when it is one of those specific admonition directives (note, attention, etc).

I could be misinterpreting how this ought to look, so let's continue the discussion.

marshallward commented 4 years ago

Sorry, there was a mistake in my previous post. I was using @idnsunset's py shortcut and did not notice that it was highlighting sourcecode. I can see that your PR does in fact restore code blocks, and perhaps all standard reST highlights.

There is still the question whether all code blocks ought to be handled this way, or just admonition blocks, but it otherwise does look quite good at the moment.

madjxatw commented 4 years ago

An admonition block just groups a couple of paragraphs, and these paragraphs are usually styled differently from regular text in exported format, e.g., outlined, shaded, or an eye-catching background color used. All the text within an admonition block should keep their semantics and get highlighted properly.

As I know, quite a few RST directives do not support directive content (body) so there is no problem with nested elements for them.

For those that support directive content, most of them should be handled the same way as admonition blocks. The exceptions I could think of right now are raw and line-block directives.

Here are some simple examples.

.. raw:: html

   raw content

Anything within a raw block will be kept verbatim in exported format, hence they should not be highlighted or highlighted in the same style as inline literals.

line-block directive has been deprecated, instead its equivalent syntax form is recommended.

.. line-block::

   line1
   line2

is equivalent to

| line1
| line2

The line-block syntax implies that only inline markups are allowed, so any other directive blocks within a line block should not be highlighted.

We could create two groups dedicated to raw and line-block to prevent them from falling back into rstExDirective group.

Refer to https://docutils.sourceforge.io/docs/ref/rst/directives.html for more info

marshallward commented 4 years ago

Sorry for the slow reply. I agree with your conclusion, that some sort of distinction ought to be made between directives with formatted bodies like admonitions (note, etc.) and others like raw without them.

It does seem that most directives containing a body expect the content to be formatted. Many do not expect a body, like image, and there may be some question how to handle these.

These are the questions that come to mind:

I think you are suggesting that we treat all text inside of directives as reST-formatted, and define exceptions for the ones which do not, such as raw or line-block. Is that right?

At some point, I'd like to go through them more carefully and see what each one does and doesn't expect.

Your current patch also highlights the unformatted content of the block, which I think looks good, but perhaps this is not what people want?

madjxatw commented 4 years ago

Any example of a directive (except for raw and line-block) that requires unformatted content text?

For directives such as image, directive content is not permitted, Sphinx will not build successfully if any directive content is given with the directive block. In other words, any text paragraph nested within a directive block that doesn't permit directive content should not be regarded as the directive content but a new RST paragraph which expects to be highlighted as normal, although it is a syntactic error.

Additional directives introduced by Sphinx should be treated in the same way as RST directives. Some of them (e.g., toctree) may need to be blacklisted like raw and line-block.

marshallward commented 4 years ago

I think you are right, there are a very few directives which do not support unformatted text. I also agree that directives which do not support a body can be ignored for now. (We don't do much to indicate explicitly incorrect syntax at the moment.)

Even line-block supports reST formatting, which I had no realized. And literal-block seems to use "verbatim" style (monospace), but otherwise respects reST highlighting. (It would be good if you could independently check this.)

As far as I can tell, these are the exceptions:

The rest are either reST formatted or do not support content.

Additional directives supported by Sphinx (or otherwise) can be handled in a "Sphinx-mode", which we've discussed in some other issues.

So your original assertion, that most blocks should be reST formatted, seems like a good one. Hopefully you don't mind me talking through it, I just wanted to establish that we wouldn't be breaking any assumptions about other directives, most of which I've never used or even looked at before!

idnsunset commented 4 years ago

The exceptions listed above do not seem to have been handled specially in the syntax file, most of them are matched against the generic rstExDirective syntax group. Here I only share my ideas about them just for reference.

I agree that the math content should be rendered as LaTex format.

The table directive is more complicated, RST supports simple table and grid table, of which the latter supports any structure in a table cell, e.g.:

.. table:: table example

   +------------+--------------------+
   |  language  | sample             |
   +============+====================+
   |  python    | .. sourcecode:: py |
   |            |                    |
   |            |    import sys      |
   +------------+--------------------+

The .. table:: line is not mandatory, it is used here for adding extra information such as table title, options. However, for the current version of the rst syntax file, the content in table cell is not highlighted correctly whether the table directive is explicitly given or not. I personally think the table directive may need to be discussed in a separate topic.

csv-table directive allows inline markups but not block markups, hence any block markups that appear in the csv table body should be highlighted normally, indicating they are not a part of the table.

list-table should be highlighted as same as list.

I have no idea what base role would make the role directive have content.

marshallward commented 4 years ago

@madjxatw I think your PR (https://github.com/madjxatw/vim-restructuredtext/commit/c267fece6febcd94673d353294f570c4d920487f) is an improvement. I've been using it for a while and have not yet seen any problems. Could you send it to the repository?

It doesn't solve every case, such as @idnsunset 's table example, but it feels much closer to what we want.

I am starting to think that rstExDirectieve should not even exist, and should just have normal unhighlighted text inside of directive blocks (excepting the cases above). But we can work on that in the future.

marshallward commented 4 years ago

Ah one thing...

The one problematic cases would seem to be math (which could potentially use all sorts of formatting tokens) and raw. Is there an easy way to exclude these?

marshallward commented 3 years ago

(Yes, I accidentally commented on the wrong issue, repeating comment below)

@madjxatw I've just applied your patch, which I should have done ages ago. Very sorry about that, sometimes I get too comfortable with the GitHub PR process.

While the particular problem has been solved, I think the discussion here is a good one and still unresolved, so I'm going to turn on Discussion and move this over there.