Merge pull request #85 from paul-gauthier/issue-82

Tell git not to quote filenames in ls-files, to handle unicode filenames
This commit is contained in:
paul-gauthier 2023-07-11 09:31:51 -07:00 committed by GitHub
commit 36625abb71
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 100 additions and 19 deletions

View file

@ -4,6 +4,7 @@
- Added `--dark-mode` to select colors suitable for a dark terminal background - Added `--dark-mode` to select colors suitable for a dark terminal background
- Reorganized the `--help` output - Reorganized the `--help` output
- Bugfix and tests around git filenames with unicode characters
- Bugfix so that aider throws an exception when OpenAI returns InvalidRequest - Bugfix so that aider throws an exception when OpenAI returns InvalidRequest
- Bugfix/improvement to /add and /drop to recurse selected directories - Bugfix/improvement to /add and /drop to recurse selected directories
- Bugfix for live diff output when using "whole" edit format - Bugfix for live diff output when using "whole" edit format

View file

@ -160,7 +160,7 @@ class Coder:
self.abs_fnames = set([str(Path(fname).resolve()) for fname in fnames]) self.abs_fnames = set([str(Path(fname).resolve()) for fname in fnames])
if self.repo: if self.repo:
rel_repo_dir = os.path.relpath(self.repo.git_dir, os.getcwd()) rel_repo_dir = self.get_rel_repo_dir()
self.io.tool_output(f"Git repo: {rel_repo_dir}") self.io.tool_output(f"Git repo: {rel_repo_dir}")
else: else:
self.io.tool_output("Git repo: none") self.io.tool_output("Git repo: none")
@ -210,6 +210,12 @@ class Coder:
self.root = utils.safe_abs_path(self.root) self.root = utils.safe_abs_path(self.root)
def get_rel_repo_dir(self):
try:
return os.path.relpath(self.repo.git_dir, os.getcwd())
except ValueError:
return self.repo.git_dir
def add_rel_fname(self, rel_fname): def add_rel_fname(self, rel_fname):
self.abs_fnames.add(self.abs_root_path(rel_fname)) self.abs_fnames.add(self.abs_root_path(rel_fname))
@ -265,7 +271,7 @@ class Coder:
new_files.append(relative_fname) new_files.append(relative_fname)
if new_files: if new_files:
rel_repo_dir = os.path.relpath(self.repo.git_dir, os.getcwd()) rel_repo_dir = self.get_rel_repo_dir()
self.io.tool_output(f"Files not tracked in {rel_repo_dir}:") self.io.tool_output(f"Files not tracked in {rel_repo_dir}:")
for fn in new_files: for fn in new_files:
@ -964,9 +970,20 @@ class Coder:
def get_tracked_files(self): def get_tracked_files(self):
if not self.repo: if not self.repo:
return [] return []
try:
commit = self.repo.head.commit
except ValueError:
return set()
files = []
for blob in commit.tree.traverse():
if blob.type == "blob": # blob is a file
files.append(blob.path)
# convert to appropriate os.sep, since git always normalizes to / # convert to appropriate os.sep, since git always normalizes to /
files = set(self.repo.git.ls_files().splitlines())
res = set(str(Path(PurePosixPath(path))) for path in files) res = set(str(Path(PurePosixPath(path))) for path in files)
return res return res
apply_update_errors = 0 apply_update_errors = 0

View file

@ -221,7 +221,8 @@ class Commands:
yield Completion(fname, start_position=-len(partial)) yield Completion(fname, start_position=-len(partial))
def glob_filtered_to_repo(self, pattern): def glob_filtered_to_repo(self, pattern):
raw_matched_files = Path(self.coder.root).glob(pattern) raw_matched_files = list(Path(self.coder.root).glob(pattern))
matched_files = [] matched_files = []
for fn in raw_matched_files: for fn in raw_matched_files:
matched_files += expand_subdir(fn.relative_to(self.coder.root)) matched_files += expand_subdir(fn.relative_to(self.coder.root))
@ -250,7 +251,10 @@ class Commands:
self.io.tool_error(f"No files to add matching pattern: {word}") self.io.tool_error(f"No files to add matching pattern: {word}")
else: else:
if Path(word).exists(): if Path(word).exists():
matched_files = [word] if Path(word).is_file():
matched_files = [word]
else:
self.io.tool_error(f"Unable to add: {word}")
elif self.io.confirm_ask( elif self.io.confirm_ask(
f"No files matched '{word}'. Do you want to create the file?" f"No files matched '{word}'. Do you want to create the file?"
): ):

View file

@ -172,7 +172,10 @@ class RepoMap:
def run_ctags(self, filename): def run_ctags(self, filename):
# Check if the file is in the cache and if the modification time has not changed # Check if the file is in the cache and if the modification time has not changed
file_mtime = os.path.getmtime(filename) file_mtime = self.get_mtime(filename)
if file_mtime is None:
return []
cache_key = filename cache_key = filename
if cache_key in self.TAGS_CACHE and self.TAGS_CACHE[cache_key]["mtime"] == file_mtime: if cache_key in self.TAGS_CACHE and self.TAGS_CACHE[cache_key]["mtime"] == file_mtime:
return self.TAGS_CACHE[cache_key]["data"] return self.TAGS_CACHE[cache_key]["data"]
@ -239,8 +242,17 @@ class RepoMap:
def save_ident_cache(self): def save_ident_cache(self):
pass pass
def get_mtime(self, fname):
try:
return os.path.getmtime(fname)
except FileNotFoundError:
self.io.tool_error(f"File not found error: {fname}")
def get_name_identifiers(self, fname, uniq=True): def get_name_identifiers(self, fname, uniq=True):
file_mtime = os.path.getmtime(fname) file_mtime = self.get_mtime(fname)
if file_mtime is None:
return set()
cache_key = fname cache_key = fname
if cache_key in self.IDENT_CACHE and self.IDENT_CACHE[cache_key]["mtime"] == file_mtime: if cache_key in self.IDENT_CACHE and self.IDENT_CACHE[cache_key]["mtime"] == file_mtime:
idents = self.IDENT_CACHE[cache_key]["data"] idents = self.IDENT_CACHE[cache_key]["data"]

View file

@ -1,8 +1,10 @@
import os
import tempfile import tempfile
import unittest import unittest
from pathlib import Path from pathlib import Path
from unittest.mock import MagicMock, patch from unittest.mock import MagicMock, patch
import git
import openai import openai
import requests import requests
@ -29,9 +31,9 @@ class TestCoder(unittest.TestCase):
coder = Coder.create(models.GPT4, None, mock_io, openai_api_key="fake_key") coder = Coder.create(models.GPT4, None, mock_io, openai_api_key="fake_key")
# Mock the git repo # Mock the git repo
mock_repo = MagicMock() mock = MagicMock()
mock_repo.git.ls_files.return_value = "file1.txt\nfile2.py" mock.return_value = set(["file1.txt", "file2.py"])
coder.repo = mock_repo coder.get_tracked_files = mock
# Call the check_for_file_mentions method # Call the check_for_file_mentions method
coder.check_for_file_mentions("Please check file1.txt and file2.py") coder.check_for_file_mentions("Please check file1.txt and file2.py")
@ -75,10 +77,9 @@ class TestCoder(unittest.TestCase):
# Initialize the Coder object with the mocked IO and mocked repo # Initialize the Coder object with the mocked IO and mocked repo
coder = Coder.create(models.GPT4, None, mock_io, openai_api_key="fake_key") coder = Coder.create(models.GPT4, None, mock_io, openai_api_key="fake_key")
# Mock the git repo mock = MagicMock()
mock_repo = MagicMock() mock.return_value = set(["file1.txt", "file2.py"])
mock_repo.git.ls_files.return_value = "file1.txt\nfile2.py" coder.get_tracked_files = mock
coder.repo = mock_repo
# Call the check_for_file_mentions method # Call the check_for_file_mentions method
coder.check_for_file_mentions("Please check file1.txt and file2.py") coder.check_for_file_mentions("Please check file1.txt and file2.py")
@ -102,10 +103,9 @@ class TestCoder(unittest.TestCase):
# Initialize the Coder object with the mocked IO and mocked repo # Initialize the Coder object with the mocked IO and mocked repo
coder = Coder.create(models.GPT4, None, mock_io, openai_api_key="fake_key") coder = Coder.create(models.GPT4, None, mock_io, openai_api_key="fake_key")
# Mock the git repo mock = MagicMock()
mock_repo = MagicMock() mock.return_value = set(["file1.txt", "other/file1.txt"])
mock_repo.git.ls_files.return_value = "file1.txt\nother/file1.txt" coder.get_tracked_files = mock
coder.repo = mock_repo
# Call the check_for_file_mentions method # Call the check_for_file_mentions method
coder.check_for_file_mentions("Please check file1.txt!") coder.check_for_file_mentions("Please check file1.txt!")
@ -352,11 +352,58 @@ class TestCoder(unittest.TestCase):
coder = Coder.create(models.GPT4, None, mock_io, openai_api_key="fake_key") coder = Coder.create(models.GPT4, None, mock_io, openai_api_key="fake_key")
# Set up the mock to raise InvalidRequestError # Set up the mock to raise InvalidRequestError
mock_chat_completion_create.side_effect = openai.error.InvalidRequestError("Invalid request", "param") mock_chat_completion_create.side_effect = openai.error.InvalidRequestError(
"Invalid request", "param"
)
# Call the run method and assert that InvalidRequestError is raised # Call the run method and assert that InvalidRequestError is raised
with self.assertRaises(openai.error.InvalidRequestError): with self.assertRaises(openai.error.InvalidRequestError):
coder.run(with_message="hi") coder.run(with_message="hi")
def test_get_tracked_files(self):
# Create a temporary directory
tempdir = Path(tempfile.mkdtemp())
# Initialize a git repository in the temporary directory and set user name and email
repo = git.Repo.init(tempdir)
repo.config_writer().set_value("user", "name", "Test User").release()
repo.config_writer().set_value("user", "email", "testuser@example.com").release()
# Create three empty files and add them to the git repository
filenames = ["README.md", "subdir/fänny.md", "systemüber/blick.md", 'file"with"quotes.txt']
created_files = []
for filename in filenames:
file_path = tempdir / filename
try:
file_path.parent.mkdir(parents=True, exist_ok=True)
file_path.touch()
repo.git.add(str(file_path))
created_files.append(Path(filename))
except OSError:
# windows won't allow files with quotes, that's ok
self.assertIn('"', filename)
self.assertEqual(os.name, "nt")
self.assertTrue(len(created_files) >= 3)
repo.git.commit("-m", "added")
# Create a Coder object on the temporary directory
coder = Coder.create(
models.GPT4,
None,
io=InputOutput(),
openai_api_key="fake_key",
fnames=[str(tempdir / filenames[0])],
)
tracked_files = coder.get_tracked_files()
# On windows, paths will come back \like\this, so normalize them back to Paths
tracked_files = [Path(fn) for fn in tracked_files]
# Assert that coder.get_tracked_files() returns the three filenames
self.assertEqual(set(tracked_files), set(created_files))
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()