Skip to content
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 for code in comments #209

Open
plevold opened this issue Oct 26, 2022 · 5 comments
Open

Syntax highlighting for code in comments #209

plevold opened this issue Oct 26, 2022 · 5 comments
Labels
enhancement New feature or request lsp/hover Issues related with hovering requests

Comments

@plevold
Copy link
Contributor

plevold commented Oct 26, 2022

Is your feature request related to a problem? Please describe.
Note: I'm not sure whether fortls or the Modern Fortran VSCode extension is the right place for this suggestion. Feel free to let me know if it should be moved. Also, it is somewhat related to #208, but I think it belongs in a separate issue.

The Rust VSCode extension / Rust-analyzer has some interesting features when it comes to comments which I think could fit nicely into Modren Fortran / fortls as well. Take the following example from the PyO3 crate:

/// A container type for (mutably) accessing [`PyClass`] values
///
/// `PyCell` autodereferences to [`PyAny`], so you can call `PyAny`'s methods on a `PyCell<T>`.
///
/// # Examples
///
/// This example demonstrates getting a mutable reference of the contained `PyClass`.
/// ```rust
/// use pyo3::prelude::*;
///
/// #[pyclass]
/// struct Number {
///     inner: u32,
/// }
///
/// #[pymethods]
/// impl Number {
///     fn increment(&mut self) {
///         self.inner += 1;
///     }
/// }
///
/// # fn main() -> PyResult<()> {
/// Python::with_gil(|py| {
///     let n = PyCell::new(py, Number { inner: 0 })?;
///
///     let n_mutable: &mut Number = &mut n.borrow_mut();
///     n_mutable.increment();
///
///     Ok(())
/// })
/// # }
/// ```
/// For more information on how, when and why (not) to use `PyCell` please see the
/// [module-level documentation](self).
#[repr(C)]
pub struct PyCell<T: PyClassImpl> {
    ob_base: <T::BaseType as PyClassBaseType>::LayoutAsBase,
    contents: PyCellContents<T>,
}

In VSCode, this renders as follows:
image

Hovering the struct (type) gives this:
image

Describe the solution you'd like
There's two nice things with the Rust-analyzer way which I think could be considered for fortls as well:

  • The code example in the documentation comment is identified and highlighted with syntax highlighting like the regular code.
  • The hover popup renders the documentation comment with syntax highlighting and rich text formatting. I think this is covered by Change hover to use MarkupContent #45 if I'm not mistaken.

Additional context
This is probably not a trivial task even if it's technically reasonable to implement. The Fortran community at large should probably agree on a way of writing documentation comments which fortls then can pick up. Rust seems to use markdown syntax for this. Another option is to use Doxygen commands like \code. Personally I like the markdown approach, but there are many considerations to be made here.

@plevold plevold added the enhancement New feature or request label Oct 26, 2022
@gnikit
Copy link
Member

gnikit commented Oct 26, 2022

I will have to think about this a bit but off the top of my head my thoughts are:

  • We should be able to inherit the markdown syntax highlighting in Modern Fortran for VS Code in comments. I am not sure how that would look like visually since comment tokens are by default coloured more or less with a single colour regardless of the underlying theme (at least for all major themes). We already do something like this for OpenMP and OpenACC
  • As you correctly pointed out this would actually be possible with Change hover to use MarkupContent #45 and trivially so. The reason why I didn't move forward with that issue is that I couldn't file a robust way to formulate markdown syntax from raw comments. The currently most used Markdown parser is not suitable for this. I think I will try this locally with manually creating the Markdown syntax and hopefully it won't look that bad.
  • If we want fortls to work within the comment docstring that will be considerably harder, since we would have to parse the comment as valid code.

@plevold
Copy link
Contributor Author

plevold commented Oct 27, 2022

  • If we want fortls to work within the comment docstring that will be considerably harder, since we would have to parse the comment as valid code.

Yes, that sounds hard... I haven't looked at the Rust-analyzer code (nor would I be competent to do that) so I can't be completely sure, but the feeling I get is that what they do is quite minimal just to get some reasonable coloring.

I had a closer look at how Rust-analyzer is behaving and made some interesting observations:

  • The comment syntax highlighting does not seem to be the exactly the same as for the code itself. For instance, some tokens are drawn with the comment text style. At line 257 in my screenshot there's even one & colored white and one which is "comment style" grey.
  • There is no hover information or goto definition support in the comment code syntax highlighting.
  • There are some links (e.g. [`PyAny`] on line 233 in my screenshot) to other structs which has hover information and goto definition support.

@gnikit
Copy link
Member

gnikit commented Oct 27, 2022

@plevold I have a "working" (tests pass but code style is hot garbage) version of the markdown LSP response. I need to do some splitting and reparsing of the docs and add a option for the deprecated style of requests i.e. where the entire hover message was put into a fortran90 markdown block.

@plevold
Copy link
Contributor Author

plevold commented Oct 28, 2022

@gnikit wow, that was quick! Feel free to ping me if you want any beta testing.

@gnikit
Copy link
Member

gnikit commented Oct 28, 2022

Splitting the documentation from within the procedure hover message is a bit of a nightmare. I'll ping you when I have something that can deploy.

@gnikit gnikit added the lsp/hover Issues related with hovering requests label Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lsp/hover Issues related with hovering requests
Projects
None yet
Development

No branches or pull requests

2 participants