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

Only listing directories that exist. #3120

Merged
merged 2 commits into from Jul 18, 2018

Conversation

okin
Copy link
Member

@okin okin commented Jul 17, 2018

Pull Request Checklist

Description

While testing Nikola 8.0.0b2 I discovered that listing installed themes does not work for me (setup with pipenv).

Traceback (most recent call last):
  File "/Users/niko/.virtualenvs/bikes.bembel.bytes-CZ5WGSwq/lib/python3.6/site-packages/doit/doit_cmd.py", line 177, in run
    return command.parse_execute(args)
  File "/Users/niko/.virtualenvs/bikes.bembel.bytes-CZ5WGSwq/lib/python3.6/site-packages/doit/cmd_base.py", line 127, in parse_execute
    return self.execute(params, args)
  File "/Users/niko/.virtualenvs/bikes.bembel.bytes-CZ5WGSwq/lib/python3.6/site-packages/nikola/plugin_categories.py", line 147, in execute
    return self._execute(options, args)
  File "/Users/niko/.virtualenvs/bikes.bembel.bytes-CZ5WGSwq/lib/python3.6/site-packages/nikola/plugins/command/theme.py", line 173, in _execute
    return self.list_installed()
  File "/Users/niko/.virtualenvs/bikes.bembel.bytes-CZ5WGSwq/lib/python3.6/site-packages/nikola/plugins/command/theme.py", line 293, in list_installed
    themes += [(i, os.path.join(tdir, i)) for i in os.listdir(tdir)]
FileNotFoundError: [Errno 2] No such file or directory: 'themes'

This PR fixes this by only listing existing directories.

@okin okin added the bug label Jul 17, 2018
@okin okin added this to the v8.0.0 milestone Jul 17, 2018
@Kwpolska
Copy link
Member

Kwpolska commented Jul 18, 2018

What if the themes directory is a symlink? I guess os.path.exists would work better.

(Unrelated, but pipenv is a bad tool, and doesn’t work well with Nikola.)

@okin
Copy link
Member Author

okin commented Jul 18, 2018

@Kwpolska os.path.isdir follows symlinks so this is not a problem. In case the link is broken this will return False.
I would like to stick with isdir because it will return False in case we unexpectedly hit a file whereas exists would accept this.

@Kwpolska Kwpolska merged commit c28147b into getnikola:master Jul 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants