Skip to content

Conversation

@vlanx
Copy link

@vlanx vlanx commented Mar 10, 2021

Description

Added a function to show the name of the python virtualenv, when its being used.

Motivation and Context

I consider having the name of the virtual environment we're in very useful.

How Has This Been Tested?

Entering and leaving a virtualenv has the desired behaviour.

Screenshots (if appropriate):

Demo

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • If my change requires a change to the documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
  • I have added tests to cover my changes, and all the new and existing tests pass.

@vlanx
Copy link
Author

vlanx commented Mar 10, 2021

If it's approved, will follow with a clean-up PR

Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

Many of the themes have their own venv detection logic, so adding one here is reasonable.

However, the detect_venv; PS1+=... approach seems outdated (although still used by some themes)

I suggest just echoing the value from within your method, leaving your PS1 definition more clean.

NOTE: I also recommend renaming detect_venv => venv_prompt

Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

I have one small comment but I think this is almost ready to go !

@davidpfarrell
Copy link
Contributor

If it's approved, will follow with a clean-up PR

It occurs to me that now might be a good time to add themes/purity to the themes section of clean_files.txt.

This will allow our internal lint-checkers to run against the theme.

@vlanx vlanx requested a review from davidpfarrell March 24, 2021 21:52
Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

This seems great to me !

@NoahGorny
Copy link
Member

Well done @tiagobarros99 and great review @davidpfarrell 😄

@NoahGorny NoahGorny merged commit 4860fb8 into Bash-it:master Mar 25, 2021
@vlanx vlanx deleted the purity-add-virtualenv branch March 29, 2021 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants