Repository: magmax/python-inquirer

Issue:
inquirer.Path rejects valid default paths
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

https://github.com/magmax/python-inquirer/blob/7ce0a8ab7d5122f140e1079b69ef40b533ee2560/src/inquirer/questions.py#L287

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:

```python
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

```python
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.

Discussion:
Cube707: @aaronhendry this version should address both issues, would you care to test it and give your opinion?

install via:
```sh
pip install -U "git+https://github.com/magmax/python-inquirer@fix/path"
```
aaronhendry: This fixes issue #585, which was the issue surrounding valid paths being rejected. It hasn't really fixed issue #586, but if you've decided to remove normalisation entirely from the Path query, then this issue isn't really relevant anymore.

Patch:
diff --git a/docs/usage.md b/docs/usage.md
index f65ed0e..c9e2fc9 100644
--- a/docs/usage.md
+++ b/docs/usage.md
@@ -227,17 +227,6 @@ By default nothing is enforced (`None`)
 Path('config_file', 'Point me to your configuration file.', exists=True, path_type=Path.File)
 ```
 
-### normalize_to_absolute_path
-
-Argument which will enable normalization on the provided path. When enabled, in case of relative path would be provided
-the Question will normalize it to absolute path.
-
-Expects `bool` value. Default `False`.
-
-```python
-Path('config_file', 'Point me to your configuration file.', normalize_to_absolute_path=True)
-```
-
 ## Creating the Question object
 
 With this information, it is easy to create a `Question` object:
diff --git a/examples/path.py b/examples/path.py
index 211ec30..f2e7568 100644
--- a/examples/path.py
+++ b/examples/path.py
@@ -17,6 +17,12 @@ questions = [
         exists=True,
         message="Give me existing file",
     ),
+    inquirer.Path(
+        "existing_dir",
+        path_type=inquirer.Path.DIRECTORY,
+        exists=True,
+        message="Give me existing dir",
+    ),
 ]
 
 answers = inquirer.prompt(questions)
diff --git a/src/inquirer/questions.py b/src/inquirer/questions.py
index e1fb427..e9f0692 100644
--- a/src/inquirer/questions.py
+++ b/src/inquirer/questions.py
@@ -2,10 +2,8 @@
 
 from __future__ import annotations
 
-import errno
 import json
-import os
-import sys
+import pathlib
 
 import inquirer.errors as errors
 from inquirer.render.console._other import GLOBAL_OTHER_CHOICE
@@ -188,44 +186,6 @@ class Checkbox(Question):
         self.autocomplete = autocomplete
 
 
-# Solution for checking valid path based on
-# https://stackoverflow.com/a/34102855/1360532
-ERROR_INVALID_NAME = 123
-
-
-def is_pathname_valid(pathname):
-    """
-    `True` if the passed pathname is a valid pathname for the current OS;
-    `False` otherwise.
-    """
-    try:
-        if not isinstance(pathname, str) or not pathname:
-            return False
-
-        _, pathname = os.path.splitdrive(pathname)
-
-        root_dirname = os.environ.get("HOMEDRIVE", "C:") if sys.platform == "win32" else os.path.sep
-
-        if not os.path.isdir(root_dirname):
-            raise Exception("'%s' is not a directory.", root_dirname)
-
-        root_dirname = root_dirname.rstrip(os.path.sep) + os.path.sep
-
-        for pathname_part in pathname.split(os.path.sep):
-            try:
-                os.lstat(root_dirname + pathname_part)
-            except OSError as exc:
-                if hasattr(exc, "winerror"):
-                    if exc.winerror == ERROR_INVALID_NAME:
-                        return False
-                elif exc.errno in {errno.ENAMETOOLONG, errno.ERANGE}:
-                    return False
-    except (ValueError, TypeError):
-        return False
-    else:
-        return True
-
-
 class Path(Text):
     ANY = "any"
     FILE = "file"
@@ -233,12 +193,15 @@ class Path(Text):
 
     kind = "path"
 
-    def __init__(self, name, default=None, path_type="any", exists=None, normalize_to_absolute_path=False, **kwargs):
+    def __init__(self, name, default=None, path_type="any", exists=None, **kwargs):
         super().__init__(name, default=default, **kwargs)
 
-        self._path_type = path_type
+        if path_type in (Path.ANY, Path.FILE, Path.DIRECTORY):
+            self._path_type = path_type
+        else:
+            raise ValueError("'path_type' must be one of [ANY, FILE, DIRECTORY]")
+
         self._exists = exists
-        self._normalize_to_absolute_path = normalize_to_absolute_path
 
         if default is not None:
             try:
@@ -246,48 +209,40 @@ class Path(Text):
             except errors.ValidationError:
                 raise ValueError("Default value '{}' is not valid based on " "your Path's criteria".format(default))
 
-    def validate(self, current):
+    def validate(self, current: str):
         super().validate(current)
 
         if current is None:
             raise errors.ValidationError(current)
 
-        current = self.normalize_value(current)
+        path = pathlib.Path(current)
+
+        # this block validates the path in correspondence with the OS
+        # it will error if the path contains invalid characters
+        try:
+            path.lstat()
+        except FileNotFoundError:
+            pass
+        except (ValueError, OSError) as e:
+            raise errors.ValidationError(e)
 
-        if not is_pathname_valid(current):
+        if (self._exists is True and not path.exists()) or (self._exists is False and path.exists()):
             raise errors.ValidationError(current)
 
         # os.path.isdir and isfile check also existence of the path,
         # which might not be desirable
-        if self._path_type == "file":
-            if self._exists is None and os.path.basename(current) == "":
-                raise errors.ValidationError(current)
-            elif self._exists and not os.path.isfile(current):
+        if self._path_type == Path.FILE:
+            if current.endswith(("\\", "/")):
                 raise errors.ValidationError(current)
-            elif self._exists is not None and not self._exists and os.path.isfile(current):
+            if path.exists() and not path.is_file():
                 raise errors.ValidationError(current)
 
-        elif self._path_type == "directory":
-            if self._exists is None and os.path.basename(current) != "":
-                raise errors.ValidationError(current)
-            elif self._exists and not os.path.isdir(current):
+        if self._path_type == Path.DIRECTORY:
+            if current == "":
                 raise errors.ValidationError(current)
-            elif self._exists is not None and not self._exists and os.path.isdir(current):
+            if path.exists() and not path.is_dir():
                 raise errors.ValidationError(current)
 
-        elif self._exists and not os.path.exists(current):
-            raise errors.ValidationError(current)
-        elif self._exists is not None and not self._exists and os.path.exists(current):
-            raise errors.ValidationError(current)
-
-    def normalize_value(self, value):
-        value = os.path.expanduser(value)
-
-        if self._normalize_to_absolute_path:
-            value = os.path.abspath(value)
-
-        return value
-
 
 def question_factory(kind, *args, **kwargs):
     for cl in (Text, Editor, Password, Confirm, List, Checkbox, Path):


Test patch:
diff --git a/tests/unit/test_question.py b/tests/unit/test_question.py
index 068061d..0c84c9d 100644
--- a/tests/unit/test_question.py
+++ b/tests/unit/test_question.py
@@ -1,7 +1,8 @@
-import os
+import pathlib
 import shutil
 import tempfile
 import unittest
+import sys
 
 from inquirer import errors
 from inquirer import questions
@@ -247,13 +248,22 @@ class TestPathQuestion(unittest.TestCase):
 
         do_test(None, False)
 
-        # Path component must not be longer then 255 bytes
-        do_test("a" * 256, False)
-        do_test(os.path.abspath("/asdf/" + "a" * 256), False)
-        do_test("{}/{}".format("a" * 255, "b" * 255), True)
-
         # Path component must not contains null bytes
-        do_test("some/path/with/{}byte".format(b"\x00".decode("utf-8")), False)
+        do_test("some/path/with/{}byte".format("\x00"), False)
+
+    @unittest.skipUnless(sys.platform.startswith("lin"), "Linux only")
+    def test_path_validation_linux(self):
+        q = questions.Path("validation_test")
+        for path in []:
+            with self.assertRaises(errors.ValidationError):
+                q.validate(path)
+
+    @unittest.skipUnless(sys.platform.startswith("win"), "Windows only")
+    def test_path_validation_windows(self):
+        q = questions.Path("validation_test")
+        for path in ["fo:/bar"]:
+            with self.assertRaises(errors.ValidationError):
+                q.validate(path)
 
     def test_path_type_validation_no_existence_check(self):
         def do_test(path_type, path, result=True):
@@ -281,75 +291,107 @@ class TestPathQuestion(unittest.TestCase):
         do_test(questions.Path.FILE, "/aa/bb")
         do_test(questions.Path.FILE, "~/aa/.bb")
 
-        do_test(questions.Path.DIRECTORY, "./aa/bb", False)
+        do_test(questions.Path.DIRECTORY, "./aa/bb")
         do_test(questions.Path.DIRECTORY, "./aa/")
-        do_test(questions.Path.DIRECTORY, "aa/bb", False)
+        do_test(questions.Path.DIRECTORY, "aa/bb")
         do_test(questions.Path.DIRECTORY, "aa/")
         do_test(questions.Path.DIRECTORY, "/aa/")
         do_test(questions.Path.DIRECTORY, "~/aa/")
-        do_test(questions.Path.DIRECTORY, "/aa/bb", False)
-        do_test(questions.Path.DIRECTORY, "~/aa/bb", False)
-
-    def test_path_type_validation_existing(self):
-        root = tempfile.mkdtemp()
-        some_existing_dir = os.path.join(root, "some_dir")
-        some_non_existing_dir = os.path.join(root, "some_non_existing_dir")
-        some_existing_file = os.path.join(root, "some_file")
-        some_non_existing_file = os.path.join(root, "some_non_existing_file")
-
-        os.mkdir(some_existing_dir)
-        open(some_existing_file, "a").close()
+        do_test(questions.Path.DIRECTORY, "/aa/bb")
+        do_test(questions.Path.DIRECTORY, "~/aa/bb")
+
+    def test_path_type_validation_existence_check(self):
+        root = pathlib.Path(tempfile.mkdtemp())
+        some_non_existing_dir = root / "some_non_existing_dir"
+        some_existing_dir = root / "some_dir"
+        some_existing_dir.mkdir()
+        some_non_existing_file = root / "some_non_existing_file"
+        some_existing_file = root / "some_file"
+        some_existing_file.touch()
 
-        def do_test(path_type, path, exists, result=True):
-            q = questions.Path("path_type_test", exists=exists, path_type=path_type)
+        def do_test(path_type, path, result=True):
+            q = questions.Path("path_type_test", exists=True, path_type=path_type)
             if result:
-                self.assertIsNone(q.validate(path))
+                self.assertIsNone(q.validate(str(path)))
             else:
                 with self.assertRaises(errors.ValidationError):
-                    q.validate(path)
+                    q.validate(str(path))
 
         try:
-            do_test(questions.Path.ANY, some_existing_file, True, True)
-            do_test(questions.Path.ANY, some_non_existing_file, True, False)
-            do_test(questions.Path.ANY, some_existing_file, False, False)
-            do_test(questions.Path.ANY, some_non_existing_file, False, True)
-            do_test(questions.Path.ANY, some_existing_dir, True, True)
-            do_test(questions.Path.ANY, some_non_existing_dir, True, False)
-            do_test(questions.Path.ANY, some_existing_dir, False, False)
-            do_test(questions.Path.ANY, some_non_existing_dir, False, True)
-
-            do_test(questions.Path.FILE, some_existing_file, True, True)
-            do_test(questions.Path.FILE, some_non_existing_file, True, False)
-            do_test(questions.Path.FILE, some_non_existing_file, False, True)
-            do_test(questions.Path.FILE, some_existing_file, False, False)
-
-            do_test(questions.Path.DIRECTORY, some_existing_dir, True, True)
-            do_test(questions.Path.DIRECTORY, some_non_existing_dir, True, False)
-            do_test(questions.Path.DIRECTORY, some_existing_dir, False, False)
-            do_test(questions.Path.DIRECTORY, some_non_existing_dir, False, True)
-
+            do_test(questions.Path.ANY, some_existing_file)
+            do_test(questions.Path.ANY, some_non_existing_file, False)
+            do_test(questions.Path.ANY, some_existing_dir)
+            do_test(questions.Path.ANY, some_non_existing_dir, False)
+
+            do_test(questions.Path.FILE, some_existing_file)
+            do_test(questions.Path.FILE, some_non_existing_file, False)
+            do_test(questions.Path.FILE, some_existing_dir, False)
+
+            do_test(questions.Path.DIRECTORY, some_existing_dir)
+            do_test(questions.Path.DIRECTORY, some_non_existing_dir, False)
+            do_test(questions.Path.DIRECTORY, some_existing_file, False)
         finally:
             shutil.rmtree(root)
 
-    def test_normalizing_value(self):
-        # Expanding Home
-        home = os.path.expanduser("~")
-        q = questions.Path("home")
+    def test_dir_and_file_same_name(self):
+        root = pathlib.Path(tempfile.mkdtemp())
+        some_file = root / "foo"
+        some_dir = root / "foo/"
 
-        path = "~/some_path/some_file"
-        self.assertNotIn(home, path)
-        self.assertIn(home, q.normalize_value(path))
+        def do_test(exists, type):
+            q = questions.Path("test", exists=exists, path_type=type)
+            with self.assertRaises(errors.ValidationError):
+                q.validate(str(some_dir))
 
-        # Normalizing to absolute path
-        root = os.path.abspath(__file__).split(os.path.sep)[0]
-        q = questions.Path("abs_path", normalize_to_absolute_path=True)
-        self.assertEqual(root, q.normalize_value("some/relative/path").split(os.path.sep)[0])
+        some_file.touch()
+        try:
+            do_test(exists=True, type=questions.Path.DIRECTORY)
+            do_test(exists=False, type=questions.Path.DIRECTORY)
+        finally:
+            some_file.unlink()
+
+        some_dir.mkdir()
+        try:
+            do_test(exists=True, type=questions.Path.FILE)
+            do_test(exists=False, type=questions.Path.FILE)
+        finally:
+            some_dir.rmdir()
 
     def test_default_value_validation(self):
+        root = pathlib.Path(tempfile.mkdtemp())
+        some_non_existing_dir = root / "some_non_existing_dir"
+        some_existing_dir = root / "some_dir"
+        some_existing_dir.mkdir()
+        some_non_existing_file = root / "some_non_existing_file"
+        some_existing_file = root / "some_file"
+        some_existing_file.touch()
+
+        def do_test(default, path_type, exists, result=True):
+            if result:
+                questions.Path("path", default=str(default), exists=exists, path_type=path_type)
+            else:
+                with self.assertRaises(ValueError):
+                    questions.Path("path", default=str(default), exists=exists, path_type=path_type)
+
+        do_test(some_existing_dir, questions.Path.DIRECTORY, exists=True)
+        do_test(some_non_existing_dir, questions.Path.DIRECTORY, exists=True, result=False)
+
+        do_test(some_existing_file, questions.Path.FILE, exists=True)
+        do_test(some_non_existing_file, questions.Path.FILE, exists=True, result=False)
+
+    def test_path_type_value_validation(self):
+        questions.Path("abs_path", path_type=questions.Path.ANY)
+        questions.Path("abs_path", path_type="any")
+        questions.Path("abs_path", path_type=questions.Path.FILE)
+        questions.Path("abs_path", path_type="file")
+        questions.Path("abs_path", path_type=questions.Path.DIRECTORY)
+        questions.Path("abs_path", path_type="directory")
+
         with self.assertRaises(ValueError):
-            questions.Path("path", default="~/.toggl_log", path_type=questions.Path.DIRECTORY)
+            questions.Path("abs_path", path_type=questions.Path.kind)
 
-        questions.Path("path", default="~/.toggl_log")
+        with self.assertRaises(ValueError):
+            questions.Path("abs_path", path_type="false")
 
 
 def test_tagged_value():