You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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.
: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
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.
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:
Pygments takes care about converting string arguments to
int
s /bool
s / etc itself, so we just pass strings there.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:
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
I wrote about "option_list", a function in your patch. I never mentioned "list options".
Look at the example again, closely:
: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:
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 justkey
. With my syntax, one can write:... 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:
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
The text was updated successfully, but these errors were encountered: