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.

Sign up for GitHub

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

Jump to bottom

Fix _legalize_path types #5224

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix _legalize_path types #5224

wants to merge 2 commits into from

Commits on May 4, 2024

  1. Fix types in beets.util.__init__

    @snejus
    snejus committed May 4, 2024
    Configuration menu
    Copy the full SHA
    adac926 View commit details
    Browse the repository at this point in the history

  2. Fix legalize_path types 

    Mypy was not happy here because `_legalize_stage` function
implementation concatenates `path` and `extension` parameters, implying
that their types need to match.

You can see that initially `path` parameter was defined as a `str` while
`extension` was `bytes`.

In reality, depending on the `fragment` parameter value, `extension` was
sometimes provided as a `str` and sometimes as `bytes`. The same
parameter decided whether `path` gets converted into `bytes` within
`_legalize_stage` implementation. No surprise that mypy was confused
here.

`_legalize_stage` is only used within `Item.destination` method
implementation which accepts where `fragment` is defined. I determined
that the `fragment` parameter controls the form of the output path:

- fragment=False returned path absolute path *as bytes* (default)
- fragment=True returned path relative to the library directory as *str*.

Given the above, this commit

1. Renames `fragment` parameter to `relative_to_libdir` for clarity
2. Makes `Item.destination` to return the same type in both cases.
   I picked `bytes` since that's the type that majority of the code
   using this method expects.

   I converted the output path to str for the code that has been
   expecting a string here.
3. Decouples `_legalize_stage` and `_legalize_path` implementations from
   the `relative_to_libdir`. The logic now uses `str` type only.
    @snejus
    snejus committed May 4, 2024
    Configuration menu
    Copy the full SHA
    079e946 View commit details
    Browse the repository at this point in the history