Virtualenv: [1168/1285] (TENTATIVE) Implement changes to prompt behavior of activate.xsh

[bskinn]

Per discussion at #1168 and #1285, as part of harmonizing the prompt handling behavior of the various supported shells, changes to activate.xsh _may_ be warranted.

xonsh has its own machinery for reporting the current virtual environment in the prompt (the {env_name} field), which is included in the default $PROMPT of the shell. Of note, as currently implemented (v0.8.8) xonsh always surrounds $VIRTUAL_ENV with parentheses when one uses the built-in {env_name} prompt-variable. Due to this machinery and its default configuration, implementation of --prompt and VIRTUAL_ENV_DISABLE_PROMPT behaviors require a rather different approach with xonsh than with the other shells.

By default, xonsh:

• Does implement default __VIRTUAL_PROMPT__ prefix of "($VIRTUAL_ENV)" (at least, if one uses the $PROMPT that ships by default with the shell).
  
  
  
  • Given that the formatting of the environment name is freely customizable by the user, I see no reason to forcibly re-implement this feature to coerce to "($VIRTUAL_ENV)".
  
  
• DOES NOT do anything at all with the argument to --prompt.
  
  
  
  • I favor implementation here, as I frequently invoke as virtualenv env --prompt="(descriptive name) ".
  
  
• DOES NOT obey VIRTUAL_ENV_DISABLE_PROMPT.
  
  
  
  • This feature could be left un-implemented, as the user can just remove {env_name} from their $PROMPT if they don't want the venv name displayed. However, I can hypothesize situations where someone would want to have {env_name} in $PROMPT in general, but be able to suppress it in certain situations. Thus, I favor implementation.
  
  

---

For the VIRTUAL_ENV_DISABLE_PROMPT behavior, the simplest approach seems to be removing the {env_name} field (if present) from the current $PROMPT:

$_OLD_XONSH_PROMPT = $PROMPT
$PROMPT = re.sub(r'\{env_name(:([^{}]*\{[^}]*\})*[^{}]*)?\}', '', $PROMPT)

(Logic of the regex to be laid out in a following comment.)

---

For the behavior of the --prompt argument, the same regex should be useful. At this time, I envision two possible behaviors: (1) the custom prompt is inserted in place of any/all instances of {env_name}, or (2) any/all instances of {env_name} are removed from $PROMPT, and the custom prompt is simply prepended to $PROMPT. Currently, I favor (1).

(1):

$_OLD_XONSH_PROMPT = $PROMPT
$PROMPT = re.sub(r'\{env_name(:([^{}]*\{[^}]*\})*[^{}]*)?\}', "__VIRTUAL_PROMPT__", $PROMPT)

(2):

$_OLD_XONSH_PROMPT = $PROMPT
$PROMPT = re.sub(r'\{env_name(:([^{}]*\{[^}]*\})*[^{}]*)?\}', '', $PROMPT)
$PROMPT = "__VIRTUAL_PROMPT__" + $PROMPT

I think it's best NOT to use a raw-string enclosure for the __VIRTUAL_PROMPT__ substitution marker, to allow users to have newlines &c. in their prompts if they want them?

Care will have to be taken in either case to sanely handle situations where __VIRTUAL_PROMPT__ contains double-quotes, if this hasn't been done already (haven't looked closely yet).

[bskinn]

Ping @scopatz, per @gaborbernat suggestion.

[bskinn]

The regex used above attempts to cover a number of likely and unlikely cases for usage of {env_name} that might occur in the wild, that activate.xsh would have to recognize in order to properly convert $PROMPT. I expect it misses some corner cases, but I think it should cover the vast majority of situations. In particular, I believe it should(?) never be possible for a sane {env_name:...} pattern to contain multiple nested curly braces, which eliminates a significant case that this regex cannot match.

r'                # Raw string, since escaping things
  \{env_name      # The field will ALWAYS start with "{env_name"
  (
    :             # If more than `{env_name:...}`, will always start with ':'
    (
      [^{}]*      # Stuff before substitution
      \{          # Opening substitution brace
      [^}]*       # Substitution format pattern (e.g., `:->12`)
      \}          # Closing substitution brace
    )*            # Can validly be 0+ formatting substitutions present
    [^{}]*        # Any remaining contents of the format pattern
  )?              # Could be just `{env_name}`, so all in here optional
  \}              # Always will be a close-brace @ end
'                 # Close the string

In manual testing, this pattern successfully matches all of the following (and yes, some of these seem rather irrational to me, but I'm trying to cover the bases):

• {env_name} - Basic pattern without only-print-if-not-None syntax
• {env_name:{} } - Pattern used in default xonsh $PROMPT, yielding the standard "($VIRTUAL_ENV)" prompt
• {env_name:{: >10} } - Pattern to set the environment name in a right-aligned field of width 10, with trailing space
• {env_name:!!!} - Pattern to provide a flag in the prompt if an environment is activated, without showing the actual name of the active environment
• {env_name:{0} {0} {0} } - Pattern that prints the environment name three times
  
  
  
  • {0} is needed here instead of {} since there's only one substitution value: ("($VIRTUAL_ENV)",)
  
  

[scopatz]

Hi there @bskinn - we can certainly change the behavior of xonsh (and cut a release) as needed. There is no reason that the {env_name} needs to be surrounded in parens, as you say. The only reason it does this is because we don't want the literal () to show up in the prompt. However, this can be routed around with more prompt functions, such as an {env_name_prefix} and {env_name_postfix}, if needed.

Implementing $VIRTUAL_ENV_DISABLE_PROMPT would also be welcome here.

I think that you understand this area a lot better than I do, so if you would like to alter xonsh here, I encourage a PR - which I'll get to promptly. If you need pointers on where to modify xonsh, just let me know!

[bskinn]

Heyo! Pleasure to e-meet you. :grinning:

Some scattershot brainstorming thoughts:

Mmmm, the possibility of tweaking the xonsh machinery never occurred to me... there are a lot of different ways to slice this, and we should probably spend a fair amount of time hammering out the best approach (greatest flexibility for the user, most robust against implementation changes in xonsh/virtualenv, most backward-compatible, etc.).

I'm somewhat hesitant to base new virtualenv prompt functionality on something that can only be relied upon in current+future xonsh versions; but, OTOH, the regex-swapping approach I've outlined above is potentially fragile to future evolution of xonsh. This is one reason why tests will be added to any & all stuff that happens here!

I do like the idea of introducing {env_name_[pre|post]fix}, since these would be a more direct/flexible way for virtualenv to hook into xonsh's machinery. The regex swapout is rather a broad hammer to bring to bear. OTOH, I feel like the regex swapout would actually introduce less interconnectivity between the two codebases (being less dependent on specific API elements), and would (presumably) also be compatible with many past versions of xonsh.

I'm not sure how $VIRTUAL_ENV_DISABLE_PROMPT behavior could be implemented from the outside except by something like the regex swapout. New code ignoring any {env_name} variables in $PROMPT when generating the prompt seems like something best sited in the xonsh codebase -- but that really cross-couples the xonsh/virtualenv codebases. Don't really like the idea of that sort of dependency.

---

SUMMARY: From the perspectives of project codebase isolation, xonsh backward compatibility, and avoiding changes to the xonsh codebase if at all possible, I think I still favor the above regex swapout approach. It leaves the control of the prompt customization in the hands of the xonsh user, while still enabling this specific set of virtualenv-idiosyncratic behaviors. My big question is whether all of this is likely to hold true, especially the emphasized part:

The regex used above attempts to cover a number of likely and unlikely cases for usage of {env_name} that might occur in the wild, that activate.xsh would have to recognize in order to properly convert $PROMPT. I expect it misses some corner cases, but I think it should cover the vast majority of situations. In particular, I believe it should(?) never be possible for a sane {env_name:...} pattern to contain multiple nested curly braces, which eliminates a significant case that this regex cannot match.

---

Totally separately, implementing the {env_name_[pre|post]fix} variables seems like a good idea, independently of how this discussion ends up.

[bskinn]

One details question, @scopatz : is there a good way for inspecting the prompt string as actually rendered for the user, not just the $PROMPT template?

[bskinn]

I'd be quite interested in your thoughts as well on how best to approach this xonsh/virtualenv interop, @asottile.

[scopatz]

Nice to e-meet you too @bskinn!

is there a good way for inspecting the prompt string as actually rendered for the user, not just the $PROMPT template?

Yes, There are a couple of different ways of doing this. If you are in Pure Python you can do:

import builtins
# option 1: property
builtins.__xonsh__.shell.prompt
# option 2: function
builtins.__xonsh__.shell.prompt_formatter(builtins.__xonsh__.env['PROMPT'])

Of course, in xonsh itself, these could just be:

# option 1: property
__xonsh__.shell.prompt
# option 2: function
__xonsh__.shell.prompt_formatter($PROMPT)

You can also render specific fields in the prompt using the $PROMPT_FIELDS variable. So if you just wanted to render {env_name}, you can.

implementing the {env_name_[pre|post]fix} variables seems like a good idea

Agreed, I'll try to implement that now.

[scopatz]

pre/post implemented in https://github.com/xonsh/xonsh/pull/2996

[bskinn]

Since xonsh and virtualenv are already coupled via $VIRTUAL_ENV, there probably isn't much harm in coupling them further to implement $VIRTUAL_ENV_DISABLE_PROMPT and --prompt.

What about something like this (code drawn from your pre/post PR; docstring would need updating):

def env_name():
    """Returns the current env_name if it non-empty, surrounded by the
    ``{env_prefix}`` and ``{env_postfix}`` fields.
    """
    env_name = find_env_name()
    if (
        builtins.__xonsh__.env.get("VIRTUAL_ENV_DISABLE_PROMPT")
        or not env_name
    ):
        # either the prompt is suppressed, or no environment; just return
        return

    # If the environment specifies a prompt, use it; else, use
    # {env_name} &c.
    venv_prompt = builtins.__xonsh__.env.get("VIRTUAL_ENV_PROMPT")
    if venv_prompt is not None:
        return venv_prompt
    else:
        pf = builtins.__xonsh__.shell.prompt_formatter
        pre = pf._get_field_value("env_prefix")
        post = pf._get_field_value("env_postfix")
        return pre + env_name + post

$VIRTUAL_ENV_PROMPT would be a new environment variable defined in activate.xsh containing the argument to --prompt. I used the default None return from .get() since someone might (for reasons obscure to me) want a specific environment not to show a prompt, but others to show one, and thus $VIRTUAL_ENV_DISABLE_PROMPT wouldn't work.

(If we could rely on 3.8, this would be a great place to use the new assignment expressions!!)

[scopatz]

Something like this would be nice!

[bskinn]

Ok, once that pre/post PR is merged, I can fork and implement, if you like?

[scopatz]

That's be awesome. @gforsyth will probably merge it soon.

[scopatz]

Ok @bskinn - that PR has been merged. Feel free to put in another!