Recreate support .gitignore PR. #102
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Sep 14, 2020
Closed
Closed
|
@hakancelik96 I've applied your suggestions to this PR! |
hakancelikdev
requested changes
Sep 14, 2020
There was a problem hiding this comment.
Hi again I did some experimentation in my local and made your codes as suitable as possible, can you follow these suggestions I will leave my diff to you.
--- a/.isort.cfg
+++ b/.isort.cfg
@@ -4,4 +4,4 @@ include_trailing_comma=True
force_grid_wrap=0
use_parentheses=True
line_length=79
-known_third_party = libcst,pathspec,semantic_version,setuptools
+known_third_party = libcst,semantic_version,setuptools
diff --git a/tests/test_session.py b/tests/test_session.py
index 35244e5..c54c185 100644
--- a/tests/test_session.py
+++ b/tests/test_session.py
@@ -2,8 +2,6 @@ import tempfile
import unittest
from pathlib import Path
-from pathspec import PathSpec
-
from unimport.session import Session
@@ -16,7 +14,7 @@ class TestSession(unittest.TestCase):
def test_list_paths_and_read(self):
for path in [Path("tests"), Path("tests/test_config.py")]:
- for p in self.session.list_paths(path, PathSpec("")):
+ for p in self.session.list_paths(path):
self.assertTrue(str(p).endswith(".py"))
def temp_refactor(self, source: str, expected: str, apply: bool = False):
diff --git a/unimport/__main__.py b/unimport/__main__.py
index ddd1e6b..99ab544 100644
--- a/unimport/__main__.py
+++ b/unimport/__main__.py
@@ -7,9 +7,11 @@ from pathlib import Path
from typing import List, Optional, Tuple, Union
try:
- from pathspec import PathSpec
-except ModuleNotFoundError:
- PathSpec = None
+ from pathspec.patterns.gitwildmatch import GitWildMatchPattern
+
+ HAS_PATHSPEC = True
+except ImportError:
+ HAS_PATHSPEC = False
import unimport.constants as C
from unimport.color import Color
@@ -69,15 +71,17 @@ def show(
)
-def get_gitignore() -> PathSpec:
- """Return a PathSpec matching gitignore content."""
- if not PathSpec:
- return None
- lines: List[str] = []
+def get_exclude_list_from_gitignore() -> List[str]:
+ """Converts .gitignore patterns to regex and return this exclude regex
+ list."""
path: Path = Path(".gitignore")
+ gitignore_regex: List[str] = []
if path.is_file():
- lines = tokenize.open(path).readlines()
- return PathSpec.from_lines("gitwildmatch", lines)
+ for line in tokenize.open(path).readlines():
+ regex = GitWildMatchPattern.pattern_to_regex(line)[0]
+ if regex:
+ gitignore_regex.append(regex)
+ return gitignore_regex
def main(argv: Optional[List[str]] = None) -> int:
@@ -192,17 +196,15 @@ def main(argv: Optional[List[str]] = None) -> int:
exclude_list.append(namespace.exclude)
if hasattr(session.config, "exclude"):
exclude_list.append(session.config.exclude) # type: ignore
- if namespace.gitignore:
- gitignore = get_gitignore()
- else:
- gitignore = None
+ if HAS_PATHSPEC and (
+ namespace.gitignore or (hasattr(session.config, "gitignore") and session.config.gitignore) # type: ignore
+ ):
+ exclude_list.extend(get_exclude_list_from_gitignore())
include = re.compile("|".join(include_list)).pattern
exclude = re.compile("|".join(exclude_list)).pattern
unused_modules = set()
for source_path in namespace.sources:
- for py_path in session.list_paths(
- source_path, gitignore, include, exclude
- ):
+ for py_path in session.list_paths(source_path, include, exclude):
session.scanner.scan(source=session.read(py_path)[0])
unused_imports = session.scanner.unused_imports
if unused_imports:
diff --git a/unimport/config.py b/unimport/config.py
index cfa1463..64eff52 100644
--- a/unimport/config.py
+++ b/unimport/config.py
@@ -1,6 +1,6 @@
import configparser
-from ast import literal_eval
from pathlib import Path
+from typing import Union
try:
import toml
@@ -36,11 +36,14 @@ class Config:
parser = configparser.ConfigParser(allow_no_value=True)
parser.read(self.config_file)
if parser.has_section(self.section):
+ get_value: Union[str, bool]
for attr in self.attrs:
get_value = parser.get(self.section, attr)
- if attr == "gitignore":
- get_value = literal_eval(get_value)
if get_value:
+ if get_value == "True":
+ get_value = True
+ elif get_value == "False":
+ get_value = False
setattr(self, attr, get_value)
def parse_toml(self) -> None:
diff --git a/unimport/session.py b/unimport/session.py
index 5289d0b..2f95e32 100644
--- a/unimport/session.py
+++ b/unimport/session.py
@@ -4,11 +4,6 @@ import tokenize
from pathlib import Path
from typing import Iterable, Iterator, Optional, Tuple
-try:
- from pathspec import PathSpec
-except ModuleNotFoundError:
- pass
-
from unimport.color import Color
from unimport.config import CONFIG_FILES, Config
from unimport.refactor import refactor_string
@@ -51,7 +46,6 @@ class Session:
def list_paths(
self,
start: Path,
- gitignore: Optional[PathSpec],
include: Optional[str] = None,
exclude: Optional[str] = None,
) -> Iterator[Path]:
@@ -64,17 +58,11 @@ class Session:
file_names = start.glob(self.GLOB_PATTERN)
else:
file_names = [start]
-
- def matcher(filename: Path) -> bool:
- return (
- bool(include_regex.search(str(filename)))
- and not bool(exclude_regex.search(str(filename)))
- and not (
- gitignore.match_file(str(filename)) if gitignore else False
- )
- )
-
- yield from filter(matcher, file_names)
+ yield from filter(
+ lambda filename: include_regex.search(str(filename))
+ and not exclude_regex.search(str(filename)),
+ file_names,
+ )
def refactor(self, source: str) -> str:
self.scanner.scan(source)
|
Updated review |
|
Sorry but I found some issues so I updated it again. |
No worries! |
|
Please wait before merging, I'll reread changes. |
|
Difference between pathspec fnmatch so it does not work as expected, if there is a solution to this I would definitely prefer |
hakancelikdev
approved these changes
Sep 14, 2020
isidentical
reviewed
Sep 14, 2020
| def get_exclude_list_from_gitignore() -> List[str]: | ||
| """Converts .gitignore patterns to regex and return this exclude regex | ||
| list.""" | ||
| path: Path = Path(".gitignore") |
There was a problem hiding this comment.
Why did you need to explicitly type this?
There was a problem hiding this comment.
Unfortunately, it's a useless type annotation!
There was a problem hiding this comment.
Anyway, I'm gonna remove it on my next PR, so thx!
I agree with you, but it seems that @hakancelik96 has policies to lower the number of normal dependencies as low as possible! ;-) |
Actually it was common policy for us, but it seems that @isidentical's ideas have changed. |
As you may well know, I've recently talked with someone who told me about why using dependencies is not a problem, so yeah, I am now +1 on dependencies. |
hakancelikdev
pushed a commit
that referenced
this pull request
Jul 16, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.