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

inquirer.Path rejects valid default paths #585

Closed
aaronhendry opened this issue Jun 19, 2024 · 1 comment · Fixed by #587
Closed

inquirer.Path rejects valid default paths #585

aaronhendry opened this issue Jun 19, 2024 · 1 comment · Fixed by #587
Labels
bug Something isn't working python Pull requests that update Python code

Comments

@aaronhendry
Copy link

The inquirer.Path question currently does not support providing a default under the following conditions:

  • path_type is set to inquirer.Path.DIRECTORY
  • normalise_to_absolute_path is set to True
  • exist is NOT set

The reason for this is due to this line

value = os.path.abspath(value)

The issue is that os.abspath removes the trailing slash from directories when converting to an absolute path. This means that a valid directory like /usr/bin/ is converted to /usr/bin, which then fails the os.path.basename(current) != "" validation check.

It's not terribly difficult to fix this, for instance:

value = os.path.abspath(value) + (os.sep if value[-1] == os.sep else "")

It's probably better to change the way this is checked, however, as there is no reason that an existing directory should need the trailing slash to make it a valid directory. You could change the existence check to

if self._exists is None:
    if os.path.exists(current):
        if not os.path.isdir(current):
            raise errors.ValidationError(current)
    elif os.path.basename(current) != "":
        raise errors.ValidationError(current)

But that makes things a bit more complicated. I guess one possibility is to consider any non-existent path as a valid directory in potentia. Most python commands will accept paths without a trailing slash as a valid directory; for instance, as long as you have write permissions, pathlib.Path('/non/existent/path'.mkdir(parents=True) is perfectly valid. In this instance it would be perfectly fine for the only failure condition to be if the file exists and it's a file, not a directory.

@Cube707 Cube707 added bug Something isn't working python Pull requests that update Python code labels Jun 19, 2024
@Cube707
Copy link
Collaborator

Cube707 commented Jun 21, 2024

I thought on this a little bit and the following makes the most sense to me:

If exists=true:

  • the file must pass os.path.isdir() or os.path.isfile() respectivly. As isdir() accepts both foo and foo/ as valid, if the directory foo/ exists, I do not think enforcing one of the inputs makes sens in this case.

if exists=false:

  • anything that can be used to create a file / directory should be valid input. As directorys can be created as foo or foo/, here again both version are acceptable.

And the additional argument: If you had to type a path into a form and it rejects your input because yo don't have a / I would get annoyed. So this should probably only be enforced if it is required to remove ambiguity.

open("foo","w").close()
inquirer.path("d", path_type=inquirer.Path.DIRECTORY)
> foo  # this should fail
> foo/ # this is accepted
> bar  # this is also acceptable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working python Pull requests that update Python code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants