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 does not normalize paths #586

Closed
aaronhendry opened this issue Jun 19, 2024 · 3 comments · Fixed by #587
Closed

inquirer.Path does not normalize paths #586

aaronhendry opened this issue Jun 19, 2024 · 3 comments · Fixed by #587

Comments

@aaronhendry
Copy link

Consider the following code:

from pprint import pprint

import inquirer
import os

if __name__ == '__main__':
    questions = [
        inquirer.Path(name='test', message="Enter valid path", path_type=inquirer.Path.DIRECTORY, exists=True,
                      normalize_to_absolute_path=True, default="~/")
    ]
    resp = inquirer.prompt(questions)
    pprint(resp)

Run this and hit enter to accept the default.

The expected output from this would be {'test': '/home/username/'}, however instead we get {'test': '~/'}. Perhaps I am misunderstanding the use-case of the normalize_to_absolute_path flag, but I was assuming it would normalize the output.

I believe the reason for this is because the validate method currently doesn't have any way of modifying the passed path. It normalizes it in order to validate it, but the original input (~/ in this case) is the value that is returned to the user. I think the "easy" solution here would be to allow the validate function of each question to return a value, such that the value could be modified as needed.

@Cube707
Copy link
Collaborator

Cube707 commented Jun 19, 2024

hm, combined with your other issue I think it would make sense to rewrite the whole thing to use pathlib.Path.

I am just debating if the returntype should change or if/how I hide that behind a flag.

@aaronhendry
Copy link
Author

aaronhendry commented Jun 19, 2024

I had that thought too, but I didn't want to put in a pull request completely rewriting it, nor suggest that the best course of action was a whole lot of work for you (or someone, at least).

I think part of the problem is that your validate method is doing double-duty, modifying the reply at the same time that it is validating it. It might make more sense to add a preprocessor function to the root Question class that is noop by default, and then override it in the Path class. Something like:

    # In the Question class
    def preprocess(self, current):
        return current
...
    # In the Path class
    @override
    def preprocess(self, current):
        if not current:
            return current
        return self.normalize_value(current)
        

That would have fairly minimal impact on the code as it is, I think. I've not spent too much time thinking about it though, so grain of salt and all that.

@Cube707
Copy link
Collaborator

Cube707 commented Jun 21, 2024

I arrived at the conclusion that inquire should always return the original input as given by the user, meaning responsibilities like normalizing the path should be up to the caller.

I have no idea what normalize_to_absolute_path was originally meant to do

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 a pull request may close this issue.

2 participants