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

Console lexer cannot detect virtual environment names #1253

Closed
goodmami opened this issue Nov 13, 2019 · 6 comments
Closed

Console lexer cannot detect virtual environment names #1253

goodmami opened this issue Nov 13, 2019 · 6 comments
Assignees
Labels
T-feature type: a new feature
Milestone

Comments

@goodmami
Copy link
Contributor

When a Python virtual environment is activated, by default it puts the virtual environment's name to the left of the prompt in a shell session. E.g.:

~$ source myenv/bin/activate
(myenv) ~$

This causes Pygments to fail to detect the prompt in the console/shell-session lexer. Compare:

Without the virtual environment:

$ pygmentize -l console -f html <<<"[~/project]$ foo -h"
<div class="highlight"><pre><span></span><span class="gp">[~/project]$</span> foo -h
</pre></div>

With the virtual environment:

$ pygmentize -l console -f html <<<"(env) [~/project]$ foo -h"
<div class="highlight"><pre><span></span><span class="go">(env) [~/project]$ foo -h</span>
</pre></div>

Note that it is detected if there is nothing after the environment name or possibly a user@host:dir without brackets:

$ pygmentize -l shell-session -f html <<<"(env)a@b:x$ foo -h"
<div class="highlight"><pre><span></span><span class="gp">(env)a@b:x$</span> foo -h
</pre></div>

Context:

Some software I develop is documented with Sphinx. Since I encourage users of my software to use virtual environments, I would like to illustrate their use in my documentation, but the lexer fails to parse my examples if I use the virtual environment names. I think this is just a matter of accounting for an optional virtual environment name in BashSessionLexer._ps1rgx, but there may be complexities I'm not considering.

@goodmami
Copy link
Contributor Author

Some more info:

There is already a \(\S+\) part of the PS1 pattern that can match virtual environment names (as long as they don't have spaces; somethng like (my env)$ would not match), but I'm not sure if that pattern is meant for some other kind of situation.

Some recommendations (e.g., Google's developer docs) say to not include the user or directory unless it's necessary. If users of Pygments follow those recommendations, no change is needed as something like (env) $ would be parsed as expected.

Some possible solutions:

  • move the \(\S+\) pattern as an optional group before anything else
  • as above, but make a new group instead of moving the existing one
  • allow a space after \(\S+\) so at least (env) user@host:dir$ would match (but (env) [~/dir]$ still would not)

@Anteru Anteru self-assigned this Nov 16, 2019
@Anteru Anteru added this to the 2.5 milestone Nov 16, 2019
@Anteru
Copy link
Collaborator

Anteru commented Nov 17, 2019

It does make sense to handle virtualenv better, but I'd like to understand what the result should be. Do you want the virtualenv part to be highlighted, or just make sure it's ignored properly?

@goodmami
Copy link
Contributor Author

Actually I'm ambivalent about whether it's (a) parsed as part of the prompt, (b) parsed and given a second class (e.g., so it could be styled differently), or (c) ignored properly. Currently it breaks the regex match, so the whole prompt fails to be colored as expected.

@Anteru
Copy link
Collaborator

Anteru commented Nov 17, 2019

Right, but it's easy to fix to ignore it (and make the prompt highlight correctly). If it should get some different class (and that is what I'm gravitating towards), then I need to figure out what class to use. Not to mention that various git integrations also mess with the console and add extra output (like (master))

@goodmami
Copy link
Contributor Author

Sorry, I have insufficient knowledge of the classes to form an opinion of which to use, but in terms of Token types it seems like it would call for a subtype of Generic.Prompt. However, this could be handled as two separate issues: first make the regex more robust (e.g., for the 2.5 release), then later find a class to assign to it. The second issue could also be expanded to use different classes for any user, host, and directory components, as well.

@Anteru Anteru added T-feature type: a new feature good first issue Good for newcomers and removed good first issue Good for newcomers labels Nov 23, 2019
Anteru added a commit that referenced this issue Nov 24, 2019
This is reported as issue #1253. This fix does not work yet -- the
parsing does not resume with the correct parser yet.
@Anteru
Copy link
Collaborator

Anteru commented Nov 24, 2019

Looking at this, it's not quite clear how this is supposed to be added. @birkenfeld : I've tried to get a venv pattern into get_tokens_unprocessed, but it's not clear to me how I get back to the normal token processing there.

Anteru added a commit that referenced this issue Nov 24, 2019
Anteru added a commit that referenced this issue Nov 24, 2019
Anteru added a commit that referenced this issue Nov 24, 2019
Anteru added a commit that referenced this issue Nov 24, 2019
Anteru added a commit that referenced this issue Nov 24, 2019
Anteru added a commit that referenced this issue Nov 24, 2019
Anteru added a commit that referenced this issue Nov 24, 2019
vkoukis added a commit to vkoukis/pygments that referenced this issue Aug 21, 2023
The default virtualenv prompt contains a single space after
the name of the virtualenv in parentheses.

Commit 3f40368 which resolved pygments#1253 assigns the "Text" class to
any whitespace between the virtualenv name and the rest of the prompt,
but this is problematic, because it makes the space selectable, and
doesn't include it in the prompt.

Fix this by assigning the Generic.Prompt class to the space between the
name of the virtualenv and the rest of the prompt, instead of Text.
Also update the test accordingly.

Signed-off-by: Vangelis Koukis <evangelos.koukis@hpe.com>
vkoukis added a commit to vkoukis/pygments that referenced this issue Aug 21, 2023
The default virtualenv prompt contains a single space after
the name of the virtualenv in parentheses.

Commit 3f40368 which resolved pygments#1253 assigns the "Text" class to
any whitespace between the virtualenv name and the rest of the prompt,
but this is problematic, because it makes the space selectable, and
doesn't include it in the prompt.

Fix this by assigning the Generic.Prompt class to the space between the
name of the virtualenv and the rest of the prompt, instead of Text.
Also update the test accordingly.

Signed-off-by: Vangelis Koukis <evangelos.koukis@hpe.com>
vkoukis added a commit to vkoukis/pygments that referenced this issue Aug 28, 2023
The default virtualenv prompt contains a single space after
the name of the virtualenv in parentheses.

Commit 3f40368 which resolved pygments#1253 assigns the "Text" class to
any whitespace between the virtualenv name and the rest of the prompt,
but this is problematic, because it makes the space selectable, and
doesn't include it in the prompt.

Fix this by assigning the Generic.Prompt class to the space between the
name of the virtualenv and the rest of the prompt, instead of Text.
Also update the test accordingly.

Closes pygments#2496

Signed-off-by: Vangelis Koukis <evangelos.koukis@hpe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-feature type: a new feature
Projects
None yet
Development

No branches or pull requests

2 participants