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

WIP: Make install editable option an option #2299

Closed
wants to merge 13 commits into from

Conversation

mlouielu
Copy link
Contributor

@mlouielu mlouielu commented Jun 3, 2018

let the -e option show in pipenv install -h and to be an option.

@uranusjr
Copy link
Member

uranusjr commented Jun 3, 2018

ref #2279

@uranusjr
Copy link
Member

uranusjr commented Jun 3, 2018

Something’s wrong :/

@techalchemy
Copy link
Member

Thanks for the PR!

the -e flag is handled in about 40 different places in the codebase, so simply adding it as a flag is not going to work out here

I wish it were this simple, but unfortunately its' not, if you want to iterate on this I'm happy to point you in the right direction

@mlouielu
Copy link
Contributor Author

mlouielu commented Jun 4, 2018

@techalchemy @uranusjr yep, I found that it is handled in multiple places in the code... I'll dig it out and iterate this PR.

@techalchemy
Copy link
Member

No worries and no rush, this one is gonna be a pain :p

@uranusjr uranusjr changed the title Make install editable option an option WIP: Make install editable option an option Jun 7, 2018
@techalchemy
Copy link
Member

does this belong in a subcommand with the multiple flag from click? That way we can have unlimited editable things?

@uranusjr
Copy link
Member

Yes, multiple=True should be added. We should probably add a test case for that usage as well.

http://click.pocoo.org/5/options/#multiple-options

@techalchemy
Copy link
Member

(you shoudl probably link to version 6)

@mlouielu
Copy link
Contributor Author

OK, I'll add the multiple and the test

@kennethreitz
Copy link
Contributor

i don't think this is neccessary, but thank you.

@uranusjr uranusjr reopened this Jul 30, 2018
@mgeisler
Copy link

i don't think this is neccessary, but thank you.

@kennethreitz, I believe it is normal to expect that pipenv install --help will list all accepted flags for pipenv install. Can you elaborate on why you say it is unnecessary?

Currently, the -e flag is not mentioned in the help and you have to look for it in the documentation: https://pipenv.readthedocs.io/en/latest/basics/#editable-dependencies-e-g-e

@kennethreitz
Copy link
Contributor

kennethreitz commented Aug 29, 2018

-e is not an argument to install, it's an argument to pip.

@kennethreitz
Copy link
Contributor

everything after 'install' is passed to pip. it's a pip feature, not a pipenv feature.

@mgeisler
Copy link

mgeisler commented Sep 3, 2018

everything after 'install' is passed to pip. it's a pip feature, not a pipenv feature.

Oh, that's interesting! I didn't know that pipenv install worked like that. Is it documented anywhere?

@mgeisler
Copy link

mgeisler commented Sep 4, 2018

I don't think the logic that is supposed to pass command line options to Pip really works as intended. Here I tried to use --no-index with pipenv install, but it tries to install a package named --no-index:

[mg]% pipenv install --no-index --find-links . foobar==1.2.3
Installing --no-index...
⠋/nix/store/lw18hsacf1h82yvwg89nn5nsns9hcxij-pipenv-2018.7.1/lib/python3.6/site-packages/pipenv/vendor/requirements/parser.py:44: UserWarning: Private repos not supported. Skipping.
  warnings.warn('Private repos not supported. Skipping.')
Traceback (most recent call last):
  ...
AttributeError: 'NoneType' object has no attribute 'specifier'

@kennethreitz
Copy link
Contributor

that's not exactly how it works

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.

5 participants