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

Support installing and uninstalling on Fish shell #1536

Merged
merged 18 commits into from
Nov 18, 2019

Conversation

justinmayer
Copy link
Contributor

Previously, the install script mentioned $PATH modifications that do not
apply to Fish. Users may have thought everything was all set up, only to
find invoking the poetry executable would fail in a Fish shell
environment because Poetry's bin directory could not be found on $PATH.

Now, if Poetry's bin directory is not found in Fish's $PATH environment
variable, that directory is added to the fish_user_paths universal
variable, making the poetry executable immediately available to
current and future Fish shell sessions.

Fixes #544

tommilligan and others added 13 commits October 10, 2019 15:07
Allow the `POETRY_HOME` environment variable to be passed during installation to change the default installation directory of `~/.poetry`:

```
POETRY_HOME=/etc/poetry python get-poetry.py
```
…1459)

* * check if relative filename is in excluded file list
* removed find_excluded_files() method from wheel.py

* added test for excluding files in wheels

* creating an own test data folder, for testing excluding files by pyproject.toml

* use as_posix() to respect windows file path delimiters
* This PR impliments the feature request python-poetry#784.

When a folder is explicit defined in `pyproject.toml` as excluded, all nested data, including subfolder, are excluded. It is no longer neccessary to use the glob `folder/**/*`

* use `Path` instead of `os.path.join` to create string for globbing

* try to fix linting error

* create glob pattern string by concatenating and not using Path

* using `os.path.isdir()`` for checking of explicit excluded name is a folder, because pathlib's `is_dir()` raises in exception under windows of name contains globing characters

* Remove nested data when wildcards where used.

Steps to do this are:
1. expand any wildcard used
2. if expanded path is a folder append  **/* and expand again

* fix linting

* only glob a second time if path is dir

* implement @sdispater 's suggestion for better readability

* fix glob for windows?

* On Windows, testing if a path with a glob is a directory will raise an OSError

* pathlibs  glob function doesn't return the correct case (https://bugs.python.org/issue26655). So switching back to  glob.glob()

* removing obsolete imports
Copy link
Contributor

@eblume eblume left a comment

Choose a reason for hiding this comment

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

This looks superb, thanks for doing this!

Copy link
Member

@kasteph kasteph left a comment

Choose a reason for hiding this comment

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

@justinmayer thank you for this PR! Could you fix the linting issue then this'll be good to merge!

@justinmayer
Copy link
Contributor Author

@stephsamson: Thanks for reviewing. As far as I can tell, that appears to be a glitch with GitHub Actions. If you look at the output, the cause shows:

##[error]fatal: couldn't find remote ref refs/pull/1535/merge

The actual linting step appears to pass without problems. Would you mind taking another look?

In order to enhance support for multiple shells in the installation
script, this moves shell type detection outside the `get_unix_profiles`
function so it can be available to multiple functions.
Previously, the install script mentioned $PATH modifications that do not
apply to Fish. Users may have thought everything was all set up, only to
find invoking the `poetry` executable would fail in a Fish shell
environment because Poetry's bin directory could not be found on $PATH.

Now, if Poetry's bin directory is not found in Fish's $PATH environment
variable, that directory is added to the `fish_user_paths` universal
variable, making the `poetry` executable immediately available to
current and future Fish shell sessions.
@justinmayer
Copy link
Contributor Author

@stephsamson: I triggered a new build, and I suspected, this was most likely a GitHub CI glitch. All tests are now passing — I think this is ready to merge.

@kasteph
Copy link
Member

kasteph commented Nov 18, 2019

Woo! Thanks @justinmayer 🚀 one more thing, it makes more sense to merge this against master instead of develop so that other fish users can have this ASAP. I'll merge it on that branch :)

@kasteph kasteph changed the base branch from develop to master November 18, 2019 09:36
@kasteph kasteph merged commit a030a5f into python-poetry:master Nov 18, 2019
@justinmayer justinmayer deleted the get_poetry_fish branch November 18, 2019 13:03
finswimmer pushed a commit to finswimmer/poetry that referenced this pull request Nov 22, 2019
Support installing and uninstalling on Fish shell
shenek pushed a commit to shenek/poetry that referenced this pull request Dec 31, 2019
Support installing and uninstalling on Fish shell
Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installation does not support the 'fish' shell
8 participants