Skip to content

Conversation

@kdeldycke
Copy link
Contributor

This PR:

  • lists all missing interactive sessions from langage REPL
  • groups sessions in OS shells or langage REPLs
  • documents the generic output lexer
  • adds session examples
  • documents the lack of ANSI support in output
  • adds pointers to third-party projects for ANSI support

Relates to:

@kdeldycke
Copy link
Contributor Author

kdeldycke commented May 29, 2023

This PR is ready to be reviewed and merged, even if the CI job on 3.12-dev is failing. See: #2445

@Anteru Anteru added the A-docs area: changes to documentation/docstrings label May 30, 2023
@Anteru
Copy link
Collaborator

Anteru commented May 30, 2023

Thanks! I'm a bit worried that the examples in the doc string will actually work with the lexer .. I'd prefer them to be also committed as test snippets. At that point however, we probably shouldn't duplicate them from the test snippets into the documentation. This could be something to solve via a private class field for the documentation, maybe something like _example = '/path/to/example' which is then parsed by the lexer page generator. This would have to go here: https://github.com/pygments/pygments/blob/master/pygments/sphinxext.py#L148

This may sound like overkill, but I think that's the best long term solution to make sure those things don't go out of sync, and it's probably (long term) a good idea to have some example file linked to every lexer.

@kdeldycke
Copy link
Contributor Author

kdeldycke commented May 31, 2023

Thanks @Anteru for the detailed feedback!

I have these kind of reflexion too with my Click Extra project, in which I have to maintain some code to keep the document and code in sync.

I have several strategies for that:

It's a little bit all over the place but does the job of reducing the cognitive work of maintenance. I'm just pointing to my stuff out as inspiration: it might not be adapted to Pygments. And I guess I know enough about Pygments and Sphinx to attempt an implementation of the long term solution you described.

But should this be done in this PR? I don't think so because of the bigger scope of that feature: it would introduce new code while this PR is only introducing rST content. So what's missing for this PR to be considered for a merge upstream?

@Anteru
Copy link
Collaborator

Anteru commented Jun 1, 2023

Honestly, I'm not sure if the added cost of having to convert this later is worth to merge this now (I'm really afraid of code rot quickly.) I'll see if I can spare some cycles to convert this, maybe it's not as complicated as I'm afraid it is. I'm not a huge fan of having this extra field on the lexer class to point to a document in the test tree, so the other alternative is to have tests somehow marked up as examples that should show up in the doc for that particular lexer. Let me think a bit more about this, and at the very least try to prototype the _example thing ... I did some work on the Sphinx extension and it feels like this shouldn't be that hard other than having to somehow pipe it through Pygments while building the docs and then insert it back into the doctree.

@kdeldycke
Copy link
Contributor Author

Since your last comment @Anteru, I realized you went ahead with the _example attribute and implemented it upstream in #2451.

So I updated this PR to use it to document all REPL sessions. All tests are passing, this PR is ready to be merged upstream! :)

@kdeldycke kdeldycke requested a review from Anteru May 18, 2024 11:58
@kdeldycke
Copy link
Contributor Author

@Anteru I just updated the PR with your requested changes, and squashed the commits. All tests are passing, this PR is ready to be merged upstream.

@Anteru Anteru merged commit a65bd36 into pygments:master May 18, 2024
@Anteru
Copy link
Collaborator

Anteru commented May 18, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-docs area: changes to documentation/docstrings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants