More robust interface to Pygments #230

Merged
basus merged 2 commits from master into master 4 years ago
basus commented 4 years ago (Migrated from github.com)
  • Clean up and collect configuration options in the Python wrapper
  • Expand wrapper interface to more flexibly set line numbers, CSS class,
    highlighted lines and encoding on each invocation
  • Update Racket side to correctly format messages to the Python wrapper

This shouldn't affect any code using Pygments. The main visible difference for Pollen users is that showing line numbers can be toggled on a per-invocation basis.

Note that the Python wrapper works a little differently that the Racket side: the Python wrapper persists most configuration between invocations, unless they are explicitly reset, while the Racket side provides the defaults every time. I'm not sure if this should be changed, because it might affect existing code, and users can probably override this in their own code if they want to.

- Clean up and collect configuration options in the Python wrapper - Expand wrapper interface to more flexibly set line numbers, CSS class, highlighted lines and encoding on each invocation - Update Racket side to correctly format messages to the Python wrapper This shouldn't affect any code using Pygments. The main visible difference for Pollen users is that showing line numbers can be toggled on a per-invocation basis. Note that the Python wrapper works a little differently that the Racket side: the Python wrapper persists most configuration between invocations, unless they are explicitly reset, while the Racket side provides the defaults every time. I'm not sure if this should be changed, because it might affect existing code, and users can probably override this in their own code if they want to.
mbutterick commented 4 years ago (Migrated from github.com)

So the public interface doesn’t change at all?

So the public interface doesn’t change at all?
basus commented 4 years ago (Migrated from github.com)

That's right. The only change on the Racket side is that the configuration commands sent to the Python wrapper is a little more verbose. No existing code using highlight needs to be touched.

That's right. The only change on the Racket side is that the configuration commands sent to the Python wrapper is a little more verbose. No existing code using `highlight` needs to be touched.
mbutterick commented 4 years ago (Migrated from github.com)

I’m afraid I couldn’t get this patch to work correctly anywhere, including the sample in the docs. None of the highlighting tags are applied:

#lang pollen
(require pollen/unstable/pygments)
◊highlight['python]{
for x in range(3):
    print x
}

Intended result:

'(div ((class "highlight")) (table ((class "sourcetable")) 
(tbody () (tr () (td ((class "linenos")) (div ((class "linenodiv")) 
(pre () "1\n2"))) (td ((class "code")) (div ((class "source")) 
(pre () (span ((class "k")) "for") " " (span ((class "n")) "x") " " 
(span ((class "ow")) "in") " " (span ((class "nb")) "range") 
(span ((class "p")) "(") (span ((class "mi")) "3") 
(span ((class "p")) "):") "\n " (span ((class "k")) "print") " " 
(span ((class "n")) "x") "\n")) "\n")))) "\n")

Actual result:

'(div ((class "highlight")) (pre (code "for x in range(3):\n    print x")))
I’m afraid I couldn’t get this patch to work correctly anywhere, including the sample [in the docs](https://docs.racket-lang.org/pollen/mini-tutorial.html#%28part._pygments-with-pollen%29). None of the highlighting tags are applied: ```racket #lang pollen ◊(require pollen/unstable/pygments) ◊highlight['python]{ for x in range(3): print x } ``` Intended result: ```racket '(div ((class "highlight")) (table ((class "sourcetable")) (tbody () (tr () (td ((class "linenos")) (div ((class "linenodiv")) (pre () "1\n2"))) (td ((class "code")) (div ((class "source")) (pre () (span ((class "k")) "for") " " (span ((class "n")) "x") " " (span ((class "ow")) "in") " " (span ((class "nb")) "range") (span ((class "p")) "(") (span ((class "mi")) "3") (span ((class "p")) "):") "\n " (span ((class "k")) "print") " " (span ((class "n")) "x") "\n")) "\n")))) "\n") ``` Actual result: ```racket '(div ((class "highlight")) (pre (code "for x in range(3):\n print x"))) ```
basus commented 4 years ago (Migrated from github.com)

Hmm... strange. On my machine I have to add #:python-executable "python3" to highlight to get it to pick up the correct version of Python, but I don't think that would be the issue if you can build examples without this patch. I ran the snippet above just in a test.html.pm file with just racket and I get:

'(root
  (div
   ((class "highlight"))
   (table
    ((class "sourcetable"))
    (tbody
     (tr
      (td ((class "linenos")) (div ((class "linenodiv")) (pre "1\n2")))
      (td
       ((class "code"))
       (div
        ((class "source"))
        (pre
         (span)
         (span ((class "k")) "for")
         " "
         (span ((class "n")) "x")
         " "
         (span ((class "ow")) "in")
         " "
         (span ((class "nb")) "range")
         (span ((class "p")) "(")
         (span ((class "mi")) "3")
         (span ((class "p")) "):")
         "\n    "
         (span ((class "nb")) "print")
         " "
         (span ((class "n")) "x")))))))))
Hmm... strange. On my machine I have to add `#:python-executable "python3"` to `highlight` to get it to pick up the correct version of Python, but I don't think that would be the issue if you can build examples without this patch. I ran the snippet above just in a `test.html.pm` file with just `racket` and I get: ```racket '(root (div ((class "highlight")) (table ((class "sourcetable")) (tbody (tr (td ((class "linenos")) (div ((class "linenodiv")) (pre "1\n2"))) (td ((class "code")) (div ((class "source")) (pre (span) (span ((class "k")) "for") " " (span ((class "n")) "x") " " (span ((class "ow")) "in") " " (span ((class "nb")) "range") (span ((class "p")) "(") (span ((class "mi")) "3") (span ((class "p")) "):") "\n " (span ((class "nb")) "print") " " (span ((class "n")) "x"))))))))) ```
mbutterick commented 4 years ago (Migrated from github.com)

On my Mac OS 10.14, python means Python 2.7. I don’t know if that’s the issue. But if so, this patch runs afoul of the Principle of Royalty.

On my Mac OS 10.14, `python` means Python 2.7. I don’t know if that’s the issue. But if so, this patch runs afoul of the [Principle of Royalty](https://github.com/mbutterick/pollen/blob/master/CONTRIBUTING.md).
basus commented 4 years ago (Migrated from github.com)

Ahh, yes I think the problems are due syntax differences between Python 2.7 and Python 3.x which affects how pipe.py is treated. I will try to see if I can come up a version that works on both 2.7 and 3.x and update accordingly.

Ahh, yes I think the problems are due syntax differences between Python 2.7 and Python 3.x which affects how `pipe.py` is treated. I will try to see if I can come up a version that works on both 2.7 and 3.x and update accordingly.
basus commented 4 years ago (Migrated from github.com)

The commit I just should pushed should have the pipe.py script working on Python 2.7. For some reason I can't get Pygments to work with Python 2.7 on my MacBook, so I can't test it.

The commit I just should pushed should have the `pipe.py` script working on Python 2.7. For some reason I can't get Pygments to work with Python 2.7 on my MacBook, so I can't test it.
mbutterick commented 4 years ago (Migrated from github.com)

OK, this patched version seems to work with 2.7.

You said this PR offers a “more robust interface” but I’m still unclear how that extra robustness is exposed to someone who might want to use it. Shouldn’t this somehow result in a change to the highlight function (e.g., extra keyword options)?

OK, this patched version seems to work with 2.7. You said this PR offers a “more robust interface” but I’m still unclear how that extra robustness is exposed to someone who might want to use it. Shouldn’t this somehow result in a change to the `highlight` function (e.g., extra keyword options)?
mbutterick commented 4 years ago (Migrated from github.com)

(FWIW if the patched version were faster than the current version, that would be another argument in its favor, but in my tests I found they were pretty much the same. What did you find?)

(FWIW if the patched version were faster than the current version, that would be another argument in its favor, but in my tests I found they were pretty much the same. What did you find?)
sorawee commented 4 years ago (Migrated from github.com)

@mbutterick as I understand, the configuration whether to show line numbers are currently shared across invocations of highlight. This PR "fixes" it by allowing different invocations to have different setting. That's why the interface doesn't really change. I would consider it to be a bug fixing PR rather than new feature.

@mbutterick as I understand, the configuration whether to show line numbers are currently _shared_ across invocations of `highlight`. This PR "fixes" it by allowing different invocations to have different setting. That's why the interface doesn't really change. I would consider it to be a bug fixing PR rather than new feature.
basus commented 4 years ago (Migrated from github.com)

Performance should be about the same. As @sorawee said, the only externally visible change is that showing line numbers can be toggled per invocation of highlight.

The other main change is that instead of configuration options being determined implicitly by the order of the first few lines of the input to the Python wrapper, each configuration line starts with a word that determines what option is being set (for example, __LINENOS__ to toggle line numbering). This isn't visible to users of the highlight function, but I think makes things a little cleaner for people who might want to write their own highlight function on the Racket side.

Performance should be about the same. As @sorawee said, the only externally visible change is that showing line numbers can be toggled per invocation of `highlight`. The other main change is that instead of configuration options being determined implicitly by the order of the first few lines of the input to the Python wrapper, each configuration line starts with a word that determines what option is being set (for example, `__LINENOS__` to toggle line numbering). This isn't visible to users of the `highlight` function, but I think makes things a little cleaner for people who might want to write their own `highlight` function on the Racket side.
mbutterick commented 4 years ago (Migrated from github.com)

Fair enough. Thanks!

Fair enough. Thanks!
The pull request has been merged as 123547f3cb.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b master master
git pull origin master

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff master
git push origin master
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: mbutterick/pollen#230
Loading…
There is no content yet.