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 formatting of relative paths #246

Merged
merged 2 commits into from
Jan 21, 2021
Merged

Fix formatting of relative paths #246

merged 2 commits into from
Jan 21, 2021

Conversation

The-Compiler
Copy link
Contributor

Prior to vulture 2.2, it was possible to pass an absolute path below the current directory, and the report showed a relative path instead:

With vulture 2.1, inside ~/proj/qutebrowser/git/:

$ vulture ~/proj/qutebrowser/git/qutebrowser/qutebrowser.py
qutebrowser/qutebrowser.py:138: unused function 'directory' (60% confidence)
qutebrowser/qutebrowser.py:186: unused function 'main' (60% confidence)

With vulture 2.3:

$ vulture ~/proj/qutebrowser/git/qutebrowser/qutebrowser.py
/home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:138: unused function 'directory' (60% confidence)
/home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:186: unused function 'main' (60% confidence)

This is due to #226, notably the path.relative_to(os.curdir) in utils.format_path - this won't work properly if the input is absolute:

>>> import os, pathlib
>>> os.getcwd()
'/tmp'
>>> pathlib.Path('/tmp/somefile').relative_to('.')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.9/pathlib.py", line 928, in relative_to
    raise ValueError("{!r} is not in the subpath of {!r}"
ValueError: '/tmp/somefile' is not in the subpath of '' OR one path is relative and the other is absolute.
>>> pathlib.Path('/tmp/somefile').relative_to(pathlib.Path.cwd())
PosixPath('somefile')

Checklist:

  • I have updated the documentation in the README.md file or my changes don't require an update.
  • I have added an entry in CHANGELOG.md.
  • I have added or adapted tests to cover my changes.

cc @ju-sh

@codecov-io
Copy link

codecov-io commented Jan 19, 2021

Codecov Report

Merging #246 (5fb9d44) into master (d40303b) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #246   +/-   ##
=======================================
  Coverage   99.36%   99.36%           
=======================================
  Files          17       17           
  Lines         631      631           
=======================================
  Hits          627      627           
  Misses          4        4           
Impacted Files Coverage Δ
vulture/utils.py 98.61% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d40303b...5fb9d44. Read the comment docs.

Prior to vulture 2.2, it was possible to pass an absolute path below the current directory, and the report showed a relative path instead:

With vulture 2.1, inside `~/proj/qutebrowser/git/`:

    $ vulture ~/proj/qutebrowser/git/qutebrowser/qutebrowser.py
    qutebrowser/qutebrowser.py:138: unused function 'directory' (60% confidence)
    qutebrowser/qutebrowser.py:186: unused function 'main' (60% confidence)

With vulture 2.3:

    $ vulture ~/proj/qutebrowser/git/qutebrowser/qutebrowser.py
    /home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:138: unused function 'directory' (60% confidence)
    /home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:186: unused function 'main' (60% confidence)

This is due to #226, notably the `path.relative_to(os.curdir)` in `utils.format_path` - this won't work properly if the input is absolute:

    >>> import os, pathlib
    >>> os.getcwd()
    '/tmp'
    >>> pathlib.Path('/tmp/somefile').relative_to('.')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.9/pathlib.py", line 928, in relative_to
        raise ValueError("{!r} is not in the subpath of {!r}"
    ValueError: '/tmp/somefile' is not in the subpath of '' OR one path is relative and the other is absolute.
    >>> pathlib.Path('/tmp/somefile').relative_to(pathlib.Path.cwd())
    PosixPath('somefile')
@The-Compiler
Copy link
Contributor Author

The-Compiler commented Jan 19, 2021

Ah, it looks like vulture turns relative paths absolute internally, so this is even an issue if passed a relative path:

$ vulture qutebrowser/qutebrowser.py
/home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:138: unused function 'directory' (60% confidence)
/home/florian/proj/qutebrowser/git/qutebrowser/qutebrowser.py:186: unused function 'main' (60% confidence)

The-Compiler added a commit to qutebrowser/qutebrowser that referenced this pull request Jan 19, 2021
@jendrikseipp jendrikseipp merged commit a017b13 into jendrikseipp:master Jan 21, 2021
@jendrikseipp
Copy link
Owner

Good catch. Thanks for taking care of this!

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.

3 participants