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

Allow other forms of "import lib" #25

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

jku
Copy link
Contributor

@jku jku commented Jan 19, 2021

Currently both of these won't be properly vendored:

import lib # pylint: disable=whatever-warning
import lib as foo

Only require any whitespace after 'lib' like the other import-form
already does.

Fixes #24


I admit this has not been tested as well as I'd like since I cannot figure out an easy way to vendor unreleased code... Is there a trick to vendoring something from a git commit or a local dir? I've tried various things like using -e git://github.com/org/project.git@commithash#egg=projectname in vendoring.txt but nothing seems to actually work (the example fails with a pip bug pypa/pip#4390).

@pradyunsg
Copy link
Owner

If you don't add the -e for the VCS installs, that should work.

@jku
Copy link
Contributor Author

jku commented Jan 20, 2021

If you don't add the -e for the VCS installs, that should work.

Cheers, that does indeed help. I've manually tested with the forms I could think of (looks good) and compared the pip vendoring results (no changes compared to vendoring master).

@pradyunsg
Copy link
Owner

pradyunsg commented Feb 2, 2021

Hiya @jku! I finally managed to make some time today to do some maintenance on this project.

Could you update this PR, and add unit tests with the same format as 4b23977?

@pradyunsg pradyunsg added the enhancement New feature or request label Feb 2, 2021
jku pushed a commit to jku/vendoring that referenced this pull request Feb 2, 2021
Especially the first regex is problematic: it's greedy in that it eats
newlines before and after but it does not allow comments or "as" keyword
after the module name.

* Don't match on newlines on both start and end (because then the next
  line can't do the same: Use re.MULTILINE instead so line starts and
  ends can be matched with ^ and $
* Make sure the first regex does not prevent other text after module
  name
* No need to match whitespace greedily when one is enough

This fixes the case:
  import a
  import a
and the case
  import a as b
and the case
  import a # with comment
and makes the regexes a little simpler to read

Fixes pradyunsg#29
Fixes pradyunsg#25
@jku jku force-pushed the fix-simple-import branch from 532d3b8 to 927af73 Compare February 2, 2021 10:19
Especially the first regex is problematic: it's greedy in that it eats
newlines before and after but it does not allow comments or "as" keyword
after the module name.

* Don't match on newlines on both start and end (because then the next
  line can't do the same: Use re.MULTILINE instead so line starts and
  ends can be matched with ^ and $
* Make sure the first regex does not prevent other text after module
  name
* No need to match whitespace greedily when one is enough

This fixes the case:
  import a
  import a
and the case
  import a as b
and the case
  import a # with comment
and makes the regexes a little simpler to read

Fixes pradyunsg#29
Fixes pradyunsg#25
@jku jku force-pushed the fix-simple-import branch from 927af73 to c0ce3a3 Compare February 2, 2021 10:22
@pradyunsg pradyunsg merged commit eafad0e into pradyunsg:master Feb 2, 2021
@pradyunsg
Copy link
Owner

Thanks @jku! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

some forms of "import mod" fail
2 participants