Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

[Enhancement] Show python version of conda virtual environment #1053

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

nipunsadvilkar
Copy link

@nipunsadvilkar nipunsadvilkar commented Nov 5, 2018

Initial setup to show conda_env segment

@nipunsadvilkar
Copy link
Author

#1019 PR here created from next branch.

@dritter : I would need your help in creating new segment. I haven't done shell scripting that much before.

Some direction would help.

@bhilburn
Copy link
Member

bhilburn commented Nov 5, 2018

@nipunsadvilkar - It looks to me like you did create a new segment! By moving the code to its own file in segments/, that's exactly what you've done.

The major items remaining for this PR, I think, are:

  1. Updating the README to reflect the new segment
  2. Adding tests for the segment.

Do you need help with either of the above?

@nipunsadvilkar
Copy link
Author

nipunsadvilkar commented Nov 5, 2018 via email

# Display the current active python version

prompt_conda_env() {
if [["${P9K_PYENV_PROMPT_ALWAYS_SHOW}" == "true" ]]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is meaningless, isn't it? If you do not set P9K_PYENV_PROMPT_ALWAYS_SHOW to true, you'll never see the segment. This would be the same, as removing it from your configuration..


prompt_conda_env() {
if [["${P9K_PYENV_PROMPT_ALWAYS_SHOW}" == "true" ]]; then
conda_py_version=$(python -c 'import sys; print(str(sys.version_info[0])+"."+str(sys.version_info[1]))')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
conda_py_version=$(python -c 'import sys; print(str(sys.version_info[0])+"."+str(sys.version_info[1]))')
local conda_py_version=$(python -c 'import sys; print(str(sys.version_info[0])+"."+str(sys.version_info[1]))')

# @description
# Display the current active python version

prompt_conda_env() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename this segment to something like prompt_python_version.. This is what it does, right? Or is it in it's current form related to conda?

# Parameters:
# segment_name context background foreground Generic Flat/Awesome-Patched Awesome-FontConfig Awesome-Mapped-FontConfig NerdFont
#   🐍 
p9k::register_segment "CONDA_ENV" "" "blue" "$DEFAULT_COLOR" '' $'\uE63C' $'\uE63C' $'\U1F40D' $'\uE73C '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
p9k::register_segment "CONDA_ENV" "" "blue" "$DEFAULT_COLOR" '' $'\uE63C' $'\uE63C' $'\U1F40D' $'\uE73C '
p9k::register_segment "PYTHON_VERSION" "" "blue" "$DEFAULT_COLOR" '' $'\uE63C' $'\uE63C' $'\U1F40D' $'\uE73C '

@dritter
Copy link
Member

dritter commented Nov 5, 2018

Regarding the tests: As this is a language segment, we need to mock python to produce predictable output. Testing this segment is similar to testing other language segments. Have a look at the tests for the php_version segment, and see how far you get with this.

@cocobear
Copy link

Why not just make a file , segments/python_version.p9k:

prompt_python_version() {
  local python_version
  python_version=$(python -V 2>&1  |awk '{print $2}')
  if [[ -n "$python_version" ]]; then
    p9k::prepare_segment "$0" "" $1 "$2" $3  "${python_version}"
  fi
}

conda virtualenv pyenv all had their own python_version ?

@jerome-diver
Copy link

@cocobear i'm thinking your idea is good and i done it on my powerlevel9k (and 10k) prompt theme.
Exactly same you said with an .zshrc option:

POWERLEVEL9K_VIRTUALENV_SHOW_VERSION=true

I also try to pull request and we will see:
My pull reauest

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants