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

Emit a warning when code-block should be pycon instead of python #11784

Closed
LLyaudet opened this issue Dec 3, 2023 · 8 comments
Closed

Emit a warning when code-block should be pycon instead of python #11784

LLyaudet opened this issue Dec 3, 2023 · 8 comments

Comments

@LLyaudet
Copy link

LLyaudet commented Dec 3, 2023

Is your feature request related to a problem? Please describe.
This PR on the repo for the packaging.python.org guide pypa/packaging.python.org#1428 reveals an easy-to-make mistake in Sphinx and similar tools:

.. code-block:: python

   >>> some Python console session

where the lexer should be pycon instead.

This looks pretty common actually:

https://grep.app/search?q=%5C.%5C.%20code-block%3A%3A%20python%5Cn%5Cs%2A%3E%3E%3E&regexp=true

https://grep.app/search?q=%60%60%60python%5Cn%3E%3E%3E&regexp=true

Describe the solution you'd like
It would be nice if a warning could be emitted in that case at doc build.

Describe alternatives you've considered
The alternative I have found is manual check, helped by commands like:

grep -r 'code-block:: python' -A 3 .

Additional context
pypa/packaging.python.org#1428
started from a correction asked here: pypa/packaging.python.org#1424
From pypa/packaging.python.org#1428,
@jeanas reported the same problem but with a distinct solution here:
pygments/pygments#2603

@LLyaudet LLyaudet changed the title <short description for the feature> Emit a warning when code-block should be pycon instead of python Dec 3, 2023
@electric-coder
Copy link

electric-coder commented Dec 4, 2023

@LLyaudet

It would be nice if a warning could be emitted in that case at doc build.

While I consider this feature would be "nice to have" I think that it's low priority.

reveals an easy-to-make mistake in Sphinx and similar tools:

An argument can be made that in most cases console examples should be included using the .. doctest:: directive (to guarantee they're correct and up-to-date during the build process), with the said directive not even having an explicit language code.

Documentation doesn't break because of a mistaken syntax highlight unlike some other open issues that render documentation either unreadable or nearly impossible. Just for comparison on Stack Overflow no one bothered supporting syntax highlight for Python REPL despite their massive userbase.

@picnixz
Copy link
Member

picnixz commented Dec 24, 2023

I don't think it's worth it. Someone on pygments argued that it'd be a hard-to-revert decision and this would overcomplicate the flow.

Personally, I just write >>> outside anything like:

blablabla
blablabl

>>> whatever

so I'm not sure it's worth effort. Instead, I would suggest to create a custom extension for that and if it has enough support, we could include it (it is more a linting thing for me, so maybe it could even be a plugin for ruff/pydocstyle/whatever). For now I'll close the issue as not planned.

@picnixz picnixz closed this as not planned Won't fix, can't repro, duplicate, stale Dec 24, 2023
@LLyaudet
Copy link
Author

Well that's not a merry Christmas action to close this like that. And it does not help free software either to put to trash good ideas. I condemn your decision and I don't think you made it with a good heart.

@picnixz
Copy link
Member

picnixz commented Dec 24, 2023

I apologize if you feel wronged but I closed it as 'not planned' because it's not planned. The use case for that is limited (at least it's not that much for me according to the stats I found on Pygments) and we shouldn't push for every feature if we can make it a separate small extension.

@LLyaudet
Copy link
Author

I don't see it that way. For me, the "not planned" status is not the "closed" status. I don't see any "Not planned" badge/tag anywhere on this issue. I just see "Closed". I don't know what is the management process for the issues in this project and if they are written somewhere. If it is the process to close what is not planned, then I can understand your decision, but I still think your process is wrong then. This issue is a "Quick win" (You should have a "Quick win" tag). If you want to onboard new contributors, you can assign it to them and they will have an easy task to do, with immediate result. It should need less than 20 lines of code to make this working, if there is already some warning display on parsing in the current code architecture. I will never understand the need to reject small steps improvements like that.

@jeanas
Copy link
Contributor

jeanas commented Dec 24, 2023

@LLyaudet A feature request being rejected on the ground of not being worth the cost is just routine, in most FOSS projects.

A useful pointer, perhaps: https://rgommers.github.io/2019/06/the-cost-of-an-open-source-contribution/

I wish you a merry Christmas nevertheless.

@picnixz
Copy link
Member

picnixz commented Dec 24, 2023

For me, the "not planned" status is not the "closed" status. I don't see any "Not planned" badge/tag anywhere on this issue. I just see "Closed"

Well, that's GitHub. I said I'll close it as 'not planned' + it's written that I closed it as 'not planned' on the history.

Actually, we do internally switch to pycon when we detect that the first line is >>>. So, we could emit a warning just before, but actually it would probably trigger a lot of build failures, especially in the following cases (assuming the default lexer is not changed or is python):

.. code-block::

   >>> ...

blabla::

  >>> ...

So, instead of using ::, people will need to switch to a verbose .. code-block:: pycon (and every build turning warnings into errors would probably face this), and honestly that's not a good idea. So, the cons would outweight the pros IMO. If you want to check that people use the correct lexer, it'd be more a linter's task rather than our task.

@LLyaudet
Copy link
Author

LLyaudet commented Dec 24, 2023

@LLyaudet A feature request being rejected on the ground of not being worth the cost is just routine, in most FOSS projects.

A useful pointer, perhaps: https://rgommers.github.io/2019/06/the-cost-of-an-open-source-contribution/

I wish you a merry Christmas nevertheless.

@jeanas Thanks for the read. I already read this kind of point of view before.
But I still don't agree. This routine is wrong.
It's not hard to create tags in GitHub like "QuickWin", "NotPlanned", "Priority:1", "Priority:2", "Priority:3", "Priority:BackLog", etc. instead of "Closed", "WeDontWantToHearFromYou", etc.

It's not only FOSS. The same happens at paid work with proprietary code:

  • An user tells me his/her use case.
  • I say "Ok. I can do that to help you (correct/enhance/add a feature)." I create a task and say "I don't know when my manager will plan it, but the need is known and the solution is sketched :)."
  • And then we redo the same tasks over and over because of sabotage, and the improvements are closed after some time by my manager.

I am... on the side of improvements.
@jeanas I wish you a merry Christmas too :)

@picnixz I still don't agree. You can display the warning when there is "python" instead of "pycon" and not display the warning when there is no qualifier of the code block, if you're anxious about too many additional work for the people building doc with sphinx. I think it is not the right way to do it, because this would not be as clean as it should be.

Seriously, if I build doc with sphinx and this warning is added. I see the problem in my doc build.
I correct all the occurrences in one commit. The build pass. It took me 2 hours maximum, and that's it.
As long as you do not have to redo things, this is easy... Seriously.

@picnixz I wish you a merry Christmas too :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants