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

Fix local path new directory #42

Merged
merged 2 commits into from
Jan 8, 2018
Merged

Fix local path new directory #42

merged 2 commits into from
Jan 8, 2018

Conversation

milin
Copy link
Contributor

@milin milin commented Jan 7, 2018

Fixes the following current issues with importanize.

  1. Running importanize on a newly created directory with a python file gives the following error
Error running importanize
Traceback (most recent call last):
  File "/Users/e002582/.virtualenvs/dtweb/lib/python2.7/site-packages/importanize/__main__.py", line 387, in main
    run(p, config, args)
  File "/Users/e002582/.virtualenvs/dtweb/lib/python2.7/site-packages/importanize/__main__.py", line 311, in run
    return run(text, config, args, source)
  File "/Users/e002582/.virtualenvs/dtweb/lib/python2.7/site-packages/importanize/__main__.py", line 261, in run
    organized = run_importanize_on_text(source, config, args)
  File "/Users/e002582/.virtualenvs/dtweb/lib/python2.7/site-packages/importanize/__main__.py", line 226, in run_im
portanize_on_text
    lines.pop(line_number)
IndexError: pop from empty list
  1. Running importanize without giving the full path freezes importanize (gets stuck in the while loop). So, if theres a project configuration return it in such cases.

@coveralls
Copy link

coveralls commented Jan 7, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling c76a481 on milin:fix_local_path_new_directory into 2e17d19 on miki725:master.

@coveralls
Copy link

coveralls commented Jan 7, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling a7d09a2 on milin:fix_local_path_new_directory into 2e17d19 on miki725:master.


while path != pathlib.Path(root or cwd.root):
Copy link
Owner

Choose a reason for hiding this comment

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

not sure when can this freeze here? this should go up the folder tree until root is encountered at which point loop should exit. what am I missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happens in cases when theres no root prefix for the specified file. for e.g
importanize utils/common.py.

path value lingers at PosixPath('.')

Copy link
Owner

Choose a reason for hiding this comment

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

ah, this method is called without resolving the path. oops

@@ -220,7 +224,8 @@ def run_importanize_on_text(text, config, args):

lines = text.splitlines()
for line_number in sorted(groups.all_line_numbers(), reverse=True):
lines.pop(line_number)
if lines:
Copy link
Owner

Choose a reason for hiding this comment

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

thanks. this bombed on on all empty files. introduced this recently

@miki725 miki725 merged commit b36d43f into miki725:master Jan 8, 2018
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.

None yet

3 participants