8000 Add support for Pygments options [SF:patches:112] · Issue #137 · chrisjsewell/docutils · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add support for Pygments options [SF:patches:112] #137

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

Open
chrisjsewell opened this issue Aug 9, 2020 · 0 comments
Open

Add support for Pygments options [SF:patches:112] #137

chrisjsewell opened this issue Aug 9, 2020 · 0 comments

Comments

@chrisjsewell
Copy link
Owner

author: mandriver
created: 2013-08-03 16:11:49.224000
assigned: None
SF_url: https://sourceforge.net/p/docutils/patches/112

This feature was originally requested in this ReText ticket.

This allows specifying options to Pygments lexers, like:

:::restructuredtext
.. code:: php(startinline=true,foo=bar)

Pygments takes care about converting string arguments to ints / bools / etc itself, so we just pass strings there.

:::diff
--- docutils/utils/code_analyzer.py	(revision 7712)
+++ docutils/utils/code_analyzer.py	(working copy)
@@ -59,11 +59,22 @@
         if not with_pygments:
             raise LexerError('Cannot analyze code. '
                                     'Pygments package not found.')
+        options = {}
+        if '(' in language and language.endswith(')'):
+            options_str = language[language.find('(')+1:-1]
+            language = language[:language.find('(')]
+            for option in options_str.split(','):
+                if len(option.split('=')) != 2:
+                    raise LexerError('Unknown option: "%s"' % option)
+                key, value = option.split('=')
+                options[key] = value
         try:
-            self.lexer = get_lexer_by_name(self.language)
+            self.lexer = get_lexer_by_name(language, **options)
         except pygments.util.ClassNotFound:
             raise LexerError('Cannot analyze code. '
                 'No Pygments lexer found for "%s".' % language)
+        except pygments.util.OptionError as e:
+            raise LexerError(e)
 
     # Since version 1.2. (released Jan 01, 2010) Pygments has a
     # TokenMergeFilter. However, this requires Python >= 2.4. When Docutils

commenter: milde
posted: 2013-08-21 22:41:57.012000
title: #112 Add support for Pygments options

I prefer the syntax suggested in http://sf.net/p/docutils/feature-requests/31.


commenter: mandriver
posted: 2013-10-04 08:58:09.987000
title: #112 Add support for Pygments options

attachments:

Sorry that it took too long, here is updated patch version.


commenter: cognifloyd
posted: 2014-06-12 15:23:07.403000
title: #112 Add support for Pygments options

Now that there's a patch for this, is there any chance of it getting accepted?


commenter: goodger
posted: 2015-01-26 19:00:09.301000
title: #112 Add support for Pygments options

-1 on the patch as it stands:

1: Looking at the patch uploaded by Dmitry Shachnev on 2013-10-04, it's now using an "option_list", which is a newly-introduced function that introduces novel syntax (one "name = value" option per line, comma-separated lists). Docutils already has syntax for these things: field lists and bullet lists. Directive options should use existing reST syntax if possible, and it's certainly possible here. For example:

.. code:: lua
   :pygments-options:
       :disabled_modules:
           * foo
           * bar
       :ensurenl: True

2: Missing tests and documentation. A documentation patch and some new unit tests would be welcome. As it is, it's a raw patch with no explanation of what it does or how it works. Code should be self-documenting, but the users of Docutils shouldn't have to read the code to figure out how to use a directive. And Docutils developers shouldn't have to do detective work to figure out what a patch does either.

Once these issues are dealt with, +0 on the idea.


commenter: mandriver
posted: 2015-01-27 19:42:00.058000
title: #112 Add support for Pygments options

Hi David,

First, there are no "list options" in Pygments, not sure what you meant by that. What my patch was doing was just passing strings for all options, and making Pygments code parse them itself.

Then, I am not sure how to implement what you suggest, because there doesn't seem to be a way to allow arbitrary names for :option:s in docutils. Of course I can bypass the API and parse that myself, but that will be an ugly hack.

That said, I don't want to do any further investigations in this area unless I know for sure that someone accepts my patch — and not ignore it for 1.5 years like it was the case until now.


commenter: cognifloyd
posted: 2015-01-28 21:14:05.890000
title: #112 Add support for Pygments options

Thanks @david for replying on the ML and here. I just cloned the repo with git-svn so that I can start trying to understand the codebase so that I might be able to help polish this patch.


commenter: goodger
posted: 2015-01-28 22:02:36.455000
title: #112 Add support for Pygments options

First, there are no "list options" in Pygments, not sure what you meant by that.

I wrote about "option_list", a function in your patch. I never mentioned "list options".

Then, I am not sure how to implement what you suggest, because there doesn't seem to be a way to allow arbitrary names for :option:s in docutils.

Look at the example again, closely:

.. code:: lua
   :pygments-options:
       :disabled_modules:
           * foo
           * bar
       :ensurenl: True

:disabled_modules: and :ensurenl: are indented relative to :pygments-options:. They comprise the content (value) of the "pygments-options" option (name), a nested field list. This can be parsed by the reST parser, resulting in name-value pairs. See the "meta" directive in docutils/parsers/rst/directives/html.py for an example of nested parsing.

Looking at the patch again, I see that the value for disabled_modules is passed as-is. Therefore the example can be simplified to:

.. code:: lua
   :pygments-options:
       :disabled_modules: foo, bar
       :ensurenl: True

As for a guarantee that your patch will be accepted: I can't give you one. I need to see a fixed, complete (docs & tests) patch first. If that means you won't do anything, so be it.

Sorry it took so long to get to it, but a silent patch is easily ignored. It took nudging from Jacob Floyd to get any action. Lesson: "the squeaky wheel gets the grease".


commenter: mandriver
posted: 2015-01-30 14:54:31.301000
title: #112 Add support for Pygments options

I still more like my initial syntax than your suggestion.

For example, Pygments allows not only key=value pairs as options, but just key. With my syntax, one can write:

:pygments-options: startinline

... and it will work. There is no equivalent with your syntax.


commenter: goodger
posted: 2015-01-30 16:35:08.232000
title: #112 Add support for Pygments options

On Fri, Jan 30, 2015 at 8:54 AM, Dmitry Shachnev mandriver@users.sf.net wrote:

I still more like my initial syntax than your suggestion.

For example, Pygments allows not only key=value pairs as options, but just
key. With my syntax, one can write:

:pygments-options: startinline

... and it will work. There is no equivalent with your syntax.

Not true:

:pygments-options: :startinline:

Flags are simply key-value pairs with an empty value.

The point of all this is, we don't add redundant syntax when existing
syntax works just fine. Consistency is a good thing. Directive options
already use field lists; nested options for downstream libraries can
use field lists too.

--
David Goodger http://python.net/~goodger

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

No branches or pull requests

1 participant
0