From 363db866a825868abcb695b50863de73cc390fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Dalfors?= Date: Tue, 1 Jul 2025 13:59:54 +0000 Subject: [PATCH] fix formatting and tests --- argmark/argmark.py | 230 +++++++++++++++++++++---------- tests/answer_subparser.md | 1 - tests/sample_subparser_script.py | 28 ++-- tests/test_argmark.py | 39 +++--- 4 files changed, 197 insertions(+), 101 deletions(-) diff --git a/argmark/argmark.py b/argmark/argmark.py index 1581cb4..26edfc2 100644 --- a/argmark/argmark.py +++ b/argmark/argmark.py @@ -2,7 +2,7 @@ import logging import os import re -from typing import List, Union +from typing import List, Union from inspect import cleandoc import sys from mdutils.mdutils import MdUtils @@ -11,8 +11,10 @@ def inline_code(text: str) -> str: return f"`{text}`" + # Helper functions for md_help + def _create_md_file_object(parser: _argparse.ArgumentParser) -> MdUtils: if parser.prog is None: logging.info("parser.prog is None, saving as foo.md") @@ -21,27 +23,37 @@ def _create_md_file_object(parser: _argparse.ArgumentParser) -> MdUtils: file_name_base = os.path.splitext(parser.prog)[0] md_file = MdUtils(file_name=file_name_base, title=parser.prog) - if parser.prog and not parser.prog.endswith(".py") and md_file.title.startswith("\n"): + if ( + parser.prog + and not parser.prog.endswith(".py") + and md_file.title.startswith("\n") + ): md_file.title = md_file.title.lstrip("\n") return md_file + def _add_parser_description(md_file: MdUtils, parser: _argparse.ArgumentParser) -> None: if parser.description: md_file.new_header(level=1, title="Description") md_file.new_paragraph(parser.description) + def _add_parser_epilog(md_file: MdUtils, parser: _argparse.ArgumentParser) -> None: if parser.epilog: md_file.new_header(level=1, title="Epilog") md_file.new_paragraph(parser.epilog) + def _add_usage_section(md_file: MdUtils, parser: _argparse.ArgumentParser) -> None: md_file.new_header(level=1, title="Usage:") usage_string = parser.format_usage() if parser.format_usage() is not None else "" md_file.insert_code(usage_string, language="bash") -def _format_action_for_table_row(action: _argparse.Action, parser: _argparse.ArgumentParser) -> List[str]: + +def _format_action_for_table_row( + action: _argparse.Action, parser: _argparse.ArgumentParser +) -> List[str]: """ Formats a single argparse.Action into a list of 4 strings for the arguments table. Handles both optional and positional arguments based on action.option_strings. @@ -49,30 +61,49 @@ def _format_action_for_table_row(action: _argparse.Action, parser: _argparse.Arg short_opt_str = "" long_opt_str = "" default_cell_str = "" - - if action.option_strings: # Optional argument - short_opts_list = [inline_code(s) for s in action.option_strings if s.startswith('-') and not s.startswith('--')] - long_opts_list = [inline_code(s) for s in action.option_strings if s.startswith('--')] + + if action.option_strings: # Optional argument + short_opts_list = [ + inline_code(s) + for s in action.option_strings + if s.startswith("-") and not s.startswith("--") + ] + long_opts_list = [ + inline_code(s) for s in action.option_strings if s.startswith("--") + ] short_opt_str = ", ".join(short_opts_list) if short_opts_list else "" long_opt_str = ", ".join(long_opts_list) if long_opts_list else "" - else: # Positional argument - short_opt_str = "" - long_opt_str = inline_code(action.dest) # Use 'dest' as the name for positional args + else: # Positional argument + short_opt_str = "" + long_opt_str = inline_code( + action.dest + ) # Use 'dest' as the name for positional args # Default string formatting - if isinstance(action, (_argparse._HelpAction, _argparse._VersionAction)) or \ - isinstance(action.default, bool) or \ - action.default == _argparse.SUPPRESS: + if ( + isinstance(action, (_argparse._HelpAction, _argparse._VersionAction)) + or isinstance(action.default, bool) + or action.default == _argparse.SUPPRESS + ): default_cell_str = "" # Specific check for required positionals (no option_strings) with no meaningful default - elif action.required and action.default is None and not action.option_strings and action.nargs is None : - default_cell_str = "" + elif ( + action.required + and action.default is None + and not action.option_strings + and action.nargs is None + ): + default_cell_str = "" elif action.default is None: default_cell_str = inline_code("None") else: - val_str = str(action.default) if isinstance(action.default, str) else repr(action.default) + val_str = ( + str(action.default) + if isinstance(action.default, str) + else repr(action.default) + ) default_cell_str = inline_code(val_str) - + # Help text string formatting if action.help is None: help_text_str = inline_code("None") @@ -81,23 +112,28 @@ def _format_action_for_table_row(action: _argparse.Action, parser: _argparse.Arg else: formatter = parser._get_formatter() help_text_str = formatter._expand_help(action).replace("\n", " ") - + return [short_opt_str, long_opt_str, default_cell_str, help_text_str] + def _build_arguments_table_data(parser: _argparse.ArgumentParser) -> List[str]: - table_rows_data: List[List[str]] = [] # Stores lists of 4 strings (each list is a row) + table_rows_data: List[List[str]] = ( + [] + ) # Stores lists of 4 strings (each list is a row) seen_action_ids = set() # First Pass (Optionals): Iterate through parser._option_string_actions.keys() # to match original iteration behavior for optionals. for option_string_key in parser._option_string_actions.keys(): action = parser._option_string_actions[option_string_key] - + if id(action) in seen_action_ids: continue - if isinstance(action, _argparse._SubParsersAction): # Skip subparser actions themselves + if isinstance( + action, _argparse._SubParsersAction + ): # Skip subparser actions themselves continue - + # This pass is primarily for optionals. # _format_action_for_table_row handles based on action.option_strings table_rows_data.append(_format_action_for_table_row(action, parser)) @@ -105,42 +141,48 @@ def _build_arguments_table_data(parser: _argparse.ArgumentParser) -> List[str]: # Second Pass (Positionals): Iterate through parser._actions for action in parser._actions: - if id(action) in seen_action_ids: # Already processed + if id(action) in seen_action_ids: # Already processed continue - if isinstance(action, _argparse._SubParsersAction): + if isinstance(action, _argparse._SubParsersAction): continue - + # If it has no option_strings, it's a positional argument if not action.option_strings: table_rows_data.append(_format_action_for_table_row(action, parser)) # No need to add to seen_action_ids here as this is the final pass for this action - + # Flatten the table_rows_data with the header final_table_list: List[str] = ["short", "long", "default", "help"] for row in table_rows_data: final_table_list.extend(row) - - logging.debug(f"Built arguments table data for parser '{parser.prog}': {final_table_list}") + + logging.debug( + f"Built arguments table data for parser '{parser.prog}': {final_table_list}" + ) return final_table_list -def _add_arguments_table(md_file: MdUtils, table_data: List[str], is_subcommand: bool = False) -> None: - level = 2 if is_subcommand else 1 - md_file.new_header(level=level, title="Arguments") - + +def _add_arguments_table( + md_file: MdUtils, table_data: List[str], is_subcommand: bool = False +) -> None: + level = 2 if is_subcommand else 1 + md_file.new_header(level=level, title="Arguments") + num_header_items = 4 num_data_rows = (len(table_data) - num_header_items) // num_header_items - - if num_data_rows <= 0: + + if num_data_rows <= 0: md_file.new_paragraph("No arguments defined for this command/subcommand.") return md_file.new_table( columns=num_header_items, - rows=num_data_rows + 1, - text=table_data, + rows=num_data_rows + 1, + text=table_data, text_align="left", ) + def gen_help(lines: List) -> None: lines_string = "import argparse\nimport argmark\n" parser_expr = re.compile(r"(\w+)\.parse_args\(") @@ -151,96 +193,131 @@ def gen_help(lines: List) -> None: if firstline_idx == -1 and "ArgumentParser(" in line: firstline_idx = i if firstline_idx != -1 and ".parse_args(" in line: - var_match = re.search(r"(\b[a-zA-Z_][a-zA-Z0-9_]*\b)\s*\.\s*parse_args\(", line) + var_match = re.search( + r"(\b[a-zA-Z_][a-zA-Z0-9_]*\b)\s*\.\s*parse_args\(", line + ) if var_match: potential_parser_var = var_match.group(1) - parser_def_segment = "\n".join(lines[firstline_idx:i+1]) - if f"{potential_parser_var} = " in parser_def_segment or f"{potential_parser_var}=" in parser_def_segment: + parser_def_segment = "\n".join(lines[firstline_idx : i + 1]) + if ( + f"{potential_parser_var} = " in parser_def_segment + or f"{potential_parser_var}=" in parser_def_segment + ): parser_var_name = potential_parser_var - lastline_idx = i - break - + lastline_idx = i + break + if firstline_idx == -1 or lastline_idx == -1 or parser_var_name is None: - logging.error("Could not robustly find ArgumentParser or the var calling .parse_args().") + logging.error( + "Could not robustly find ArgumentParser or the var calling .parse_args()." + ) return - - script_segment_for_parser_def = cleandoc("\n".join(lines[firstline_idx:lastline_idx])) - final_exec_string = f"{script_segment_for_parser_def}\nargmark.md_help({parser_var_name})" - + + script_segment_for_parser_def = cleandoc( + "\n".join(lines[firstline_idx:lastline_idx]) + ) + final_exec_string = ( + f"{script_segment_for_parser_def}\nargmark.md_help({parser_var_name})" + ) + exec_globals = { - "argparse": _argparse, "argmark": sys.modules[__name__], "__name__": "__main__" } + "argparse": _argparse, + "argmark": sys.modules[__name__], + "__name__": "__main__", + } logging.debug(f"Executing for gen_help:\n{final_exec_string}") try: exec(final_exec_string, exec_globals) except Exception as e: - logging.error(f"Error executing for gen_help: {e}\nCode:\n{final_exec_string}", exc_info=True) + logging.error( + f"Error executing for gen_help: {e}\nCode:\n{final_exec_string}", + exc_info=True, + ) + def md_help(parser: _argparse.ArgumentParser) -> None: md_file = _create_md_file_object(parser) - if parser.prog and md_file.title == parser.prog: + if parser.prog and md_file.title == parser.prog: md_file.new_header(level=1, title=parser.prog) - + _add_parser_description(md_file, parser) _add_parser_epilog(md_file, parser) _add_usage_section(md_file, parser) - - main_table_data = _build_arguments_table_data(parser) - - if len(main_table_data) > 4: + + main_table_data = _build_arguments_table_data(parser) + + if len(main_table_data) > 4: _add_arguments_table(md_file, main_table_data, is_subcommand=False) else: - md_file.new_header(level=1, title="Arguments") - md_file.new_paragraph("No command-line arguments defined (excluding default help).") - logging.info(f"No arguments to document in the table for parser '{parser.prog}'.") + md_file.new_header(level=1, title="Arguments") + md_file.new_paragraph( + "No command-line arguments defined (excluding default help)." + ) + logging.info( + f"No arguments to document in the table for parser '{parser.prog}'." + ) subparsers_action = None for action in parser._actions: if isinstance(action, _argparse._SubParsersAction): subparsers_action = action break - - if subparsers_action and hasattr(subparsers_action, 'choices') and subparsers_action.choices: - md_file.new_header(level=1, title="Subcommands") + + if ( + subparsers_action + and hasattr(subparsers_action, "choices") + and subparsers_action.choices + ): + md_file.new_header(level=1, title="Subcommands") for name, sub_parser_instance in subparsers_action.choices.items(): md_file.new_header(level=2, title=f"Subcommand: {inline_code(name)}") - + if sub_parser_instance.description: - _add_parser_description(md_file, sub_parser_instance) + _add_parser_description(md_file, sub_parser_instance) if sub_parser_instance.epilog: - _add_parser_epilog(md_file, sub_parser_instance) - - _add_usage_section(md_file, sub_parser_instance) + _add_parser_epilog(md_file, sub_parser_instance) + + _add_usage_section(md_file, sub_parser_instance) sub_table_data = _build_arguments_table_data(sub_parser_instance) if len(sub_table_data) > 4: - _add_arguments_table(md_file, sub_table_data, is_subcommand=True) + _add_arguments_table(md_file, sub_table_data, is_subcommand=True) else: - md_file.new_header(level=2, title="Arguments") - md_file.new_paragraph(f"No arguments defined for subcommand {inline_code(name)}.") - md_file.new_paragraph("---") + md_file.new_header(level=2, title="Arguments") + md_file.new_paragraph( + f"No arguments defined for subcommand {inline_code(name)}." + ) + md_file.new_paragraph("---") md_file.create_md_file() - md_path = md_file.file_name if md_file.file_name.endswith(".md") else md_file.file_name + ".md" + md_path = ( + md_file.file_name + if md_file.file_name.endswith(".md") + else md_file.file_name + ".md" + ) with open(md_path, "a", encoding="utf-8") as f: if not open(md_path, "rb").read().endswith(b"\n"): f.write("\n") + def main(): - script_argv = [arg for arg in sys.argv[1:] if arg != '--'] + script_argv = [arg for arg in sys.argv[1:] if arg != "--"] parser = _argparse.ArgumentParser( prog="argmark", description="Convert argparse based bin scripts to markdown documents", formatter_class=_argparse.ArgumentDefaultsHelpFormatter, ) parser.add_argument( - "-f", "--files", help="files to convert", required=True, nargs="+") - parser.add_argument( - "-v", "--verbose", help="Be verbose", action="store_true") + "-f", "--files", help="files to convert", required=True, nargs="+" + ) + parser.add_argument("-v", "--verbose", help="Be verbose", action="store_true") args, unknown_args = parser.parse_known_args(script_argv) - logging_format = "%(asctime)s - %(funcName)s -%(name)s - %(levelname)s - %(message)s" + logging_format = ( + "%(asctime)s - %(funcName)s -%(name)s - %(levelname)s - %(message)s" + ) if args.verbose: logging.basicConfig(level=logging.DEBUG, format=logging_format) else: @@ -260,5 +337,6 @@ def main(): # Removed the general Exception catch to be more specific as per instructions # If other errors need to be caught, they can be added here. + if __name__ == "__main__": main() diff --git a/tests/answer_subparser.md b/tests/answer_subparser.md index 73c683d..4ec9351 100644 --- a/tests/answer_subparser.md +++ b/tests/answer_subparser.md @@ -1,4 +1,3 @@ - app === diff --git a/tests/sample_subparser_script.py b/tests/sample_subparser_script.py index 388a153..8a12cbd 100644 --- a/tests/sample_subparser_script.py +++ b/tests/sample_subparser_script.py @@ -1,20 +1,32 @@ import argparse parser = argparse.ArgumentParser(prog="app", description="Main app with subparsers.") -parser.add_argument('--verbose', action='store_true', help='Enable verbose output.') +parser.add_argument("--verbose", action="store_true", help="Enable verbose output.") -subparsers = parser.add_subparsers(title='subcommands', dest='command', help='Available subcommands', required=True) +subparsers = parser.add_subparsers( + title="subcommands", dest="command", help="Available subcommands", required=True +) # Subparser for 'command1' -parser_cmd1 = subparsers.add_parser('command1', help='Short help for command1.', description="Detailed desc for command1") -parser_cmd1.add_argument('--opt1', type=int, default=10, help='Option for command1.') -parser_cmd1.add_argument('pos1', help='Positional arg for command1.') +parser_cmd1 = subparsers.add_parser( + "command1", + help="Short help for command1.", + description="Detailed desc for command1", +) +parser_cmd1.add_argument("--opt1", type=int, default=10, help="Option for command1.") +parser_cmd1.add_argument("pos1", help="Positional arg for command1.") # Subparser for 'command2' -parser_cmd2 = subparsers.add_parser('command2', help='Short help for command2.', epilog="Epilog for command2") # No explicit description -parser_cmd2.add_argument('--flag', action='store_true', help='A boolean flag for command2.') +parser_cmd2 = subparsers.add_parser( + "command2", help="Short help for command2.", epilog="Epilog for command2" +) # No explicit description +parser_cmd2.add_argument( + "--flag", action="store_true", help="A boolean flag for command2." +) # Call parse_args directly for gen_help to correctly identify the parser variable. # The if __name__ == '__main__' block caused indentation issues with how gen_help constructs its exec string. -args = parser.parse_args([]) # Pass empty list to avoid issues with actual CLI args during testing/analysis +args = parser.parse_args( + [] +) # Pass empty list to avoid issues with actual CLI args during testing/analysis # print(args) # Not needed for argmark diff --git a/tests/test_argmark.py b/tests/test_argmark.py index 263feca..86eb962 100644 --- a/tests/test_argmark.py +++ b/tests/test_argmark.py @@ -1,7 +1,7 @@ import filecmp import sys -import logging # Added import -import os # Already present by implication of _install_dir +import logging # Added import +import os # Already present by implication of _install_dir from argmark.argmark import * @@ -26,10 +26,10 @@ def test_gen_help(): def test_main_file_not_found_error_handling(caplog): """Tests that main() handles FileNotFoundError gracefully.""" non_existent_file = "this_file_does_not_exist_ever.py" - + # Ensure the non-existent file really doesn't exist before the test if os.path.exists(non_existent_file): - os.remove(non_existent_file) # Should not happen with this name + os.remove(non_existent_file) # Should not happen with this name original_argv = sys.argv # Simulate command line arguments for argmark's main() @@ -40,17 +40,17 @@ def test_main_file_not_found_error_handling(caplog): caplog.set_level(logging.ERROR) try: - main() # Call the main function from argmark.argmark + main() # Call the main function from argmark.argmark except SystemExit as e: - # argparse by default calls sys.exit() on error. + # argparse by default calls sys.exit() on error. # If main() calls parser.error() or if required args aren't met, # it might exit. For a FileNotFoundError, our handler shouldn't cause SystemExit. # However, if no files are processed successfully, main() might not produce output, # which could be fine. The key is it doesn't crash from an unhandled FileNotFoundError. # For this test, we are checking if our try/except in main() for open() works. - pass # Or assert e.code if a specific exit code is expected for "no files processed" + pass # Or assert e.code if a specific exit code is expected for "no files processed" - sys.argv = original_argv # Restore original sys.argv + sys.argv = original_argv # Restore original sys.argv # Verify that an error message was logged assert len(caplog.records) >= 1, "No error message was logged." @@ -59,11 +59,16 @@ def test_main_file_not_found_error_handling(caplog): if record.levelname == "ERROR" and non_existent_file in record.message: # Check if the message contains 'File not found' or the specific exception text # Example: "Error: File not found: this_file_does_not_exist_ever.py. [Errno 2] No such file or directory: 'this_file_does_not_exist_ever.py'" - if "File not found" in record.message or "No such file or directory" in record.message: + if ( + "File not found" in record.message + or "No such file or directory" in record.message + ): found_log = True break - assert found_log, f"Expected error message for {non_existent_file} not found in logs: {caplog.text}" - + assert ( + found_log + ), f"Expected error message for {non_existent_file} not found in logs: {caplog.text}" + # Clean up just in case, though it should not have been created if os.path.exists(non_existent_file): os.remove(non_existent_file) @@ -72,23 +77,25 @@ def test_main_file_not_found_error_handling(caplog): def test_gen_help_with_subparsers(): py_file = os.path.join(_install_dir, "sample_subparser_script.py") # Output prog is "app", so md_file will be "app.md" - md_file = "app.md" + md_file = "app.md" answer_file = os.path.join(_install_dir, "answer_subparser.md") - + # Ensure old files are removed if any if os.path.exists(md_file): os.remove(md_file) with open(py_file, "r") as f: gen_help(f.readlines()) - + assert os.path.isfile(md_file), f"{md_file} was not generated." # For debugging if filecmp fails: # with open(md_file, 'r') as f_actual, open(answer_file, 'r') as f_expected: # print("Actual output:\n", f_actual.read()) # print("Expected output:\n", f_expected.read()) - assert filecmp.cmp(md_file, answer_file, shallow=False), f"Generated {md_file} does not match {answer_file}." - + assert filecmp.cmp( + md_file, answer_file, shallow=False + ), f"Generated {md_file} does not match {answer_file}." + if os.path.exists(md_file): os.remove(md_file)