🐛 FIX: Glob path works as intended (#35)

[Joinyy]

Replace click.Path with the click-path GlobPath to have the CLI work as intended in the readme.

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

[codecov]

Codecov Report

Base: 84.40% // Head: 84.40% // No change to project coverage 👍

Coverage data is based on head (e6d2fa6) compared to base (6e51d18).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #39   +/-   ##
=======================================
  Coverage   84.40%   84.40%           
=======================================
  Files           9        9           
  Lines        1629     1629           
=======================================
  Hits         1375     1375           
  Misses        254      254           

Flag	Coverage Δ
pytests	84.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[chrisjsewell]

Heya, thanks for the PR!

to have the CLI work as intended in the readme.

Erm when you say it doesn't work, how are you using it?

For unix/osx terminals they already handle the glob expansion, e.g.:

rst-to-myst$ echo docs/**/*.rst
docs/source/api.rst docs/source/cli.rst

So it certainly works for me as intended:

rst-to-myst/$ rst2myst convert docs/**/*.rst
docs/source/api.rst -> docs/source/api.md
CONVERTED (extensions: [])
docs/source/cli.rst -> docs/source/cli.md
CONVERTED (extensions: [])

[chrisjsewell]

Also, looking at the code for the added extension, its very minimal: https://gitlab.com/abraxos/click-path/-/blob/master/click_path/core.py#L10

So if we were to do something like this, I would rather just have the code, and avoid the extra dependency 😅

[chrisjsewell]

ah I see its meant to fix for windows, but yeh I wouldadd the code here for the paramtype, and also would be good to have a test in tests/test_cli.py

[Joinyy]

Yes exactly, it does currently not work on windows except if you specify every file separately. Theoretically this should maybe be fixed within the click package, as the click.Path behaves different on Linux and Windows…
After the weekend I will remove the additional dependency and just put in the type directly as well as a test case 😊

[Joinyy]

So I have done some tests and even though it is fixable with the above mentionned code there seems to be another issue here. While testing on a different laptop, I could not get the previously mentionned error on windows, there I used python 3.8.5. After installing python 3.10.7 (newest) the error was reproducible again. This is due to python 3.10 installing click in the version 7.1.2 by default (don't know why...). When installing the newest click (8.3.1) specifically it works perfectly fine like it is on the main branch, so there no work would be needed even on windows.
I would suggest to bump the click version in the pyproject.toml to 8.1.3, as this will fix the error / enables the click.Path to work on windows like it was intended by click in the first place.

[chrisjsewell]

This is due to python 3.10 installing click in the version 7.1.2 by default (don't know why...)

yes it would be nice to know why this 🤔