Skip to content

Commit

Permalink
Refactor run command and enhance error handling
Browse files Browse the repository at this point in the history
- Improved 'run' function readability in 'run.py'.
- 'list_files' and 'list_names' now raise ValueError for invalid inputs.
- These functions also raise ValueError when path is not found.
- 'list_names' now directly iterates over returned names.
- Updated test case in 'test_namespace.py' for these changes.
  • Loading branch information
basicthinker committed Aug 28, 2023
1 parent 20e0e7d commit e3b7511
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 13 deletions.
7 changes: 3 additions & 4 deletions devchat/_cli/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
help='List all commands in JSON format.')
@click.option('--recursive', '-r', 'recursive_flag', is_flag=True, default=True,
help='List all commands recursively.')
def run(command: str, list_flag: bool, recursive_flag: bool):
def run(command: str, list_flag: bool, recursive_flag: bool):
"""
Operate workflow engine of DevChat.
Operate the workflow engine of DevChat.
"""
_, _, user_chat_dir = init_dir()
with handle_errors():
Expand All @@ -33,9 +33,8 @@ def run(command: str, list_flag: bool, recursive_flag: bool):
commander = CommandParser(namespace)

if list_flag:
names = namespace.list_names(command, recursive_flag)
commands = []
for name in names:
for name in namespace.list_names(command, recursive_flag):
cmd = commander.parse(name)
if not cmd:
logger.warning("Existing command directory failed to parse: %s", name)
Expand Down
18 changes: 10 additions & 8 deletions devchat/engine/namespace.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ def get_file(self, name: str, file: str) -> Optional[str]:
# If no file is found, return None
return None

def list_files(self, name: str) -> Optional[List[str]]:
def list_files(self, name: str) -> List[str]:
"""
:param name: The command name in the namespace.
:return: The full paths of the files in the command directory.
"""
if not self.is_valid_name(name):
return None
raise ValueError(f"Invalid name to list files: {name}")
# Convert the dot-separated name to a path
path = os.path.join(*name.split('.'))
files = {}
Expand All @@ -70,21 +70,21 @@ def list_files(self, name: str) -> Optional[List[str]]:
path_found = True
for file in os.listdir(full_path):
files[file] = os.path.join(full_path, file)
# If no existing path is found, return None
# If no existing path is found, raise an error
if not path_found:
return None
raise ValueError(f"Path not found to list files: {name}")
# If path is found but no files exist, return an empty list
# Sort the files in alphabetical order before returning
return sorted(files.values()) if files else []

def list_names(self, name: str = '', recursive: bool = False) -> Optional[List[str]]:
def list_names(self, name: str = '', recursive: bool = False) -> List[str]:
"""
:param name: The command name in the namespace. Defaults to the root.
:param recursive: Whether to list all descendant names or only child names.
:return: A list of all names under the given name, or None if the name is invalid.
:return: A list of all names under the given name.
"""
if not self.is_valid_name(name):
return None
raise ValueError(f"Invalid name to list names: {name}")
commands = set()
path = os.path.join(*name.split('.'))
found = False
Expand All @@ -95,7 +95,9 @@ def list_names(self, name: str = '', recursive: bool = False) -> Optional[List[s
self._add_dirnames_to_commands(full_path, name, commands)
if recursive:
self._add_recursive_dirnames_to_commands(full_path, name, commands)
return sorted(commands) if found else None
if not found:
raise ValueError(f"Path not found to list names: {name}")
return sorted(commands)

def _add_dirnames_to_commands(self, full_path: str, name: str, commands: set):
for dirname in os.listdir(full_path):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_namespace.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_list_files(tmp_path):

# Test case 3: a path exists but has no files
os.makedirs(os.path.join(tmp_path, 'org', 'd', 'e', 'f'), exist_ok=True)
assert namespace.list_files('d.e.f') == []
assert not namespace.list_files('d.e.f')

# Test case 4: a path that exists in a later branch
# Create a file in the 'sys' branch
Expand Down

0 comments on commit e3b7511

Please sign in to comment.