linenums support for inline option #652

lightxue · 2018-04-04T22:23:51Z

Signed-off-by: lightxue [email protected]

linenums in codehilite only supports 3 options now:

None
True
False

In fact, in pygments, linenos supports a string option: inline。

https://github.com/nex3/pygments/blob/df106dc8c0ff67fdc6369265f6cb4e6832698792/pygments/formatters/html.py#L352-L359

Signed-off-by: lightxue <[email protected]>

waylan · 2018-04-04T22:46:43Z

I wasn't aware of this option so I checked the docs:

If set to 'table', output line numbers as a table with two cells, one containing the line numbers, the other the whole code. This is copy-and-paste-friendly, but may cause alignment problems with some browsers or fonts. If set to 'inline', the line numbers will be integrated in the <pre> tag that contains the code (that setting is new in Pygments 0.8).

For compatibility with Pygments 0.7 and earlier, every true value except 'inline' means the same as 'table' (in particular, that means also True).

The default value is False, which means no line numbers at all.

I wonder if inline should be the new default. I very much dislike table.

waylan

If a change is necessary, then we need a test to go with it. And the docs need to be updated.

waylan · 2018-04-04T22:53:49Z

markdown/extensions/codehilite.py

@@ -249,6 +249,8 @@ def __init__(self, **kwargs):
                             'Default: True']
            }

+        if kwargs.get('linenums') == 'inline':
+            self.config['linenums'][0] = kwargs['linenums']
        super(CodeHiliteExtension, self).__init__(**kwargs)


This should be completely unnecessary. The call to super(CodeHiliteExtension, self).__init__(**kwargs) should already pass on the value. In fact, I don't see any reason to make any changes at all. Did you check whether this works before making the change?

markdown/markdown/extensions/__init__.py

Line 46 in a4d4b61

value = parseBoolValue(value, preserve_none=True)

I checked. It would raise a ValueError if linenums is a string.

Ah right. So then you should just need to change the type to accept strings.

markdown/markdown/util.py

Lines 100 to 116 in a4d4b61

def parseBoolValue(value, fail_on_errors=True, preserve_none=False):

"""Parses a string representing bool value. If parsing was successful,

returns True or False. If preserve_none=True, returns True, False,

or None. If parsing was not successful, raises ValueError, or, if

fail_on_errors=False, returns None."""

if not isinstance(value, string_type):

if preserve_none and value is None:

return value

return bool(value)

elif preserve_none and value.lower() == 'none':

return None

elif value.lower() in ('true', 'yes', 'y', 'on', '1'):

return True

elif value.lower() in ('false', 'no', 'n', 'off', '0', 'none'):

return False

elif fail_on_errors:

raise ValueError('Cannot parse bool value: %r' % value)

Only changing the type to be string would cause compatibility issue. Maybe someone wrote 'yes' in his code and expected linenums to be True.

But if you update the docs, only allow True, False, None, 'inline', it will be nice.

So I forgot how types were defined. But it's documented:

Sets a configuration setting for key with the given value. If key is unknown, a KeyError is raised. If the previous value of key was a Boolean value, then value is converted to a Boolean value. If the previous value of key is None, then value is converted to a Boolean value except when it is None. No conversion takes place when the previous value of key is a string.

So your change is a hack to change the existing value to a string and avoid calling parseBoolValue. I don't like the hack, but this is the problem with implied types.

Yes, it is a ugly way to avoid compatibility issue. The root of all this is linenos in pygments accept two types. Maybe there is a elegant way to fix it.

What about add a new parameter lineno_format with two options 'table' and 'inline'?

What about add a new parameter lineno_format with two options 'table' and 'inline'?

That might be a reasonable solution. And the value would be ignored except when linenums is True.

mitya57 · 2018-04-05T06:33:28Z

I wonder if inline should be the new default. I very much dislike table.

With inline copy-pasting won't work properly. So I would rather leave the current default.

facelessuser · 2018-06-16T03:49:47Z

I've been experimenting with Pygments inline in SuperFences. I'm not suggesting we should or shouldn't include inline output in CodeHilite, only showcasing what I've been experimenting with (I myself am still debating whether to include in my own package).

Anyways, there is a way to do inline and still be able to select, and copy and paste in a sane manner. By default, Pygments does something like this for inline line numbers:

<span class="linenos">   1</span>

This means that when you copy and paste from the code block, you will pick up line numbers which can be annoying.

But if you alter the output to:

<span class="linenos" data-linenos="   1"></span>

You can use CSS and display line numbers that don't get included in a copy/paste.

[data-linenos]:before {
    content: attr(data-linenos);
    /* some additional padding and/or margin may be required. */
}

Now you can have inline output with sane selection and copy/paste:

The subclassing of the Pygments HtmlFormatter object is pretty straight forward: facelessuser/pymdown-extensions@a67a3e7#diff-e331ec6f621f208fd178913907efa563R118.

So, if inline was a preferred, or at least a desirable output option, it could be done in a similar fashion. I generally don't like Pygments default inline output because of the bad copy/paste side effects.

Just some food for thought while considering adding inline.

waylan · 2018-06-17T00:04:56Z

@facelessuser that’s really cool. The problem is that we suggest users use Pygments default styles which won’t work without modification. That being the case this should probably be an option rather than the default.

facelessuser · 2018-06-17T00:51:08Z

The problem is that we suggest users use Pygments default styles which won’t work without modification.

Understood, and when suggesting it, I realized this was probably the case. For those willing to tweak CSS, it's probably the only way to get copy/paste and inline. Figured at the very least it would be interested to some 🙂 .

LennonChin · 2018-06-22T03:44:06Z

@facelessuser that's a cool way, but I am puzzled that if some lines have no line number, the styles of these lines seem to be not easy to control.

facelessuser · 2018-06-22T04:06:18Z

@facelessuser that's a cool way, but I am puzzled that if some lines have no line number, the styles of these lines seem to be not easy to control.

I'm not sure exactly what you are referring to, but in general, they work just as well as the default inline, only the line numbers are easily excluded from copy/paste. You can have lines with no numbers just fine, and style them however you like:

You can see it just generates the empty lines with empty padded data:

<pre><span></span><span class="lineno" data-linenos="2 "></span>"""Some file."""
<span class="lineno" data-linenos="  "></span>import foo.bar
<span class="lineno" data-linenos="4 "></span>import boo.baz
<span class="lineno" data-linenos="  "></span>import foo.bar.baz
</pre>

waylan · 2018-07-24T18:51:32Z

What about add a new parameter lineno_format with two options table and inline?

That might be a reasonable solution. And the value would be ignored except when linenums is True.

I will merge this if the above change is made and some tests are added.

waylan · 2019-11-26T19:24:43Z

This is being closed in favor of #822 which adds support for all options, not just one.

* Add `language-` prefix to output when syntax highlighting is disabled for both codehilite and fenced_code extensions. * Add `lang_prefix` config option to customize the prefix. * Add a 'pygments' env to tox which runs the tests with Pygments installed. Pygments is locked to a specific version in the env. * Updated codehilite to accept any Pygments options. * Refactor fenced code attributes. - ID attr is defined on `pre` tag. - Add support for attr_list extension, which allows setting arbitrary attributes. - When syntax highlighting is enabled, any pygments options can be defined per block in the attr list. - For backward compatibility, continue to support `hi_lines` outside of an attr_list. That is the only attr other than lang which is allowed without the brackets (`{}`) of an attr list. Note that if the brackets exist, then everything, including lang and hl_lines, must be within them. * Resolves #775. Resolves #334. Addresses #652.

linenums support for inline option

aee2628

Signed-off-by: lightxue <[email protected]>

waylan requested changes Apr 4, 2018

View reviewed changes

waylan added feature Feature request. extension Related to one or more of the included extensions. requires-changes Awaiting updates after a review. labels Oct 23, 2018

p-ouellette mentioned this pull request May 11, 2019

CodeHilite: Support Pygments options #822

Closed

waylan mentioned this pull request Sep 30, 2019

Always wrap CodeHilite code in <code> tags #862

Merged

4 tasks

waylan closed this Nov 26, 2019

waylan added rejected The pull request is rejected for the stated reasons. and removed requires-changes Awaiting updates after a review. labels Nov 26, 2019

waylan mentioned this pull request Jun 14, 2020

Support for pygments’ lineanchors option. #980

Closed

	def parseBoolValue(value, fail_on_errors=True, preserve_none=False):
	"""Parses a string representing bool value. If parsing was successful,
	returns True or False. If preserve_none=True, returns True, False,
	or None. If parsing was not successful, raises ValueError, or, if
	fail_on_errors=False, returns None."""
	if not isinstance(value, string_type):
	if preserve_none and value is None:
	return value
	return bool(value)
	elif preserve_none and value.lower() == 'none':
	return None
	elif value.lower() in ('true', 'yes', 'y', 'on', '1'):
	return True
	elif value.lower() in ('false', 'no', 'n', 'off', '0', 'none'):
	return False
	elif fail_on_errors:
	raise ValueError('Cannot parse bool value: %r' % value)

linenums support for inline option #652

linenums support for inline option #652

Uh oh!

Conversation

lightxue commented Apr 4, 2018

Uh oh!

waylan commented Apr 4, 2018

Uh oh!

waylan left a comment

Choose a reason for hiding this comment

Uh oh!

waylan Apr 4, 2018

Choose a reason for hiding this comment

Uh oh!

lightxue Apr 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waylan Apr 4, 2018

Choose a reason for hiding this comment

Uh oh!

lightxue Apr 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waylan Apr 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lightxue Apr 4, 2018

Choose a reason for hiding this comment

Uh oh!

lightxue Apr 4, 2018

Choose a reason for hiding this comment

Uh oh!

waylan Apr 5, 2018

Choose a reason for hiding this comment

Uh oh!

mitya57 commented Apr 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facelessuser commented Jun 16, 2018

Uh oh!

waylan commented Jun 17, 2018

Uh oh!

facelessuser commented Jun 17, 2018

Uh oh!

LennonChin commented Jun 22, 2018

Uh oh!

facelessuser commented Jun 22, 2018

Uh oh!

waylan commented Jul 24, 2018

Uh oh!

waylan commented Nov 26, 2019

Uh oh!

Uh oh!

lightxue Apr 4, 2018 •

edited

Loading

lightxue Apr 4, 2018 •

edited

Loading

waylan Apr 4, 2018 •

edited

Loading

mitya57 commented Apr 5, 2018 •

edited

Loading