From 82f053e625073668f0cb8cb70f4ef453ba0b876e Mon Sep 17 00:00:00 2001 From: Kevin Wurster Date: Thu, 10 Oct 2024 13:42:21 -0400 Subject: [PATCH 1/8] Better CLI metavars --- pyin.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pyin.py b/pyin.py index b09f181..d3918a9 100644 --- a/pyin.py +++ b/pyin.py @@ -1079,7 +1079,7 @@ def argparse_parser(): input_group = aparser.add_mutually_exclusive_group() input_group.add_argument( '--gen', - metavar='expression', + metavar='EXPR', dest='generate_expr', type=_type_gen, help="Execute this Python expression and feed results into other" @@ -1087,7 +1087,7 @@ def argparse_parser(): ) input_group.add_argument( '-i', '--infile', - metavar='path', + metavar='PATH', type=argparse.FileType('r'), default='-', help="Read input from this file. Use '-' for stdin (the default)." @@ -1095,7 +1095,7 @@ def argparse_parser(): aparser.add_argument( '-o', '--outfile', - metavar='path', + metavar='PATH', type=argparse.FileType('w'), default='-', help="Write to this file. Use '-' for stdout (the default)." @@ -1107,13 +1107,13 @@ def argparse_parser(): help=f"Write this after every line. Defaults to: {repr(os.linesep)}." ) aparser.add_argument( - '-s', '--setup', action='append', + '-s', '--setup', action='append', metavar='EXPR', help="Execute one or more Python statements to pre-initialize objects," " import objects with new names, etc." ) aparser.add_argument( '--variable', - metavar='string', + metavar='EXPR', type=_type_variable, default=_DEFAULT_VARIABLE, help="Place each input item in this variable when evaluating" @@ -1121,7 +1121,7 @@ def argparse_parser(): ) aparser.add_argument( '--stream-variable', - metavar='string', + metavar='STR', type=_type_variable, default=_DEFAULT_STREAM_VARIABLE, help="Place the stream in this variable when evaluating expressions" @@ -1130,7 +1130,7 @@ def argparse_parser(): aparser.add_argument( 'expressions', - metavar='expressions', + metavar='EXPR', nargs='*', help='Python expression.' ) From 56fd8abfa72244197bfa850b7e88d16452ffa0db Mon Sep 17 00:00:00 2001 From: Kevin Wurster Date: Thu, 10 Oct 2024 13:43:04 -0400 Subject: [PATCH 2/8] Optionally print the full traceback --- pyin.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pyin.py b/pyin.py index d3918a9..6d19c9e 100644 --- a/pyin.py +++ b/pyin.py @@ -1326,17 +1326,19 @@ def _cli_entrypoint(rawargs=None): print() # Don't get a trailing newline otherwise exit_code = 128 + signal.SIGINT - # A 'RuntimeError()' indicates a problem that should have been caught - # during testing. We want a full traceback in these cases. - except RuntimeError: # pragma no cover - print(''.join(traceback.format_exc()).rstrip(), file=sys.stderr) - exit_code = 1 - - # Generic error reporting except Exception as e: - print("ERROR:", str(e), file=sys.stderr) + exit_code = 1 + # A 'RuntimeError()' indicates a problem that should have been caught + # during testing. We want a full traceback in these cases. + if 'PYIN_FULL_TB' in os.environ or isinstance(e, RuntimeError): + message = ''.join(traceback.format_exc()).rstrip() + else: + message = f"ERROR: {str(e)}" + + print(message, file=sys.stderr) + # If the input and/or file points to a file descriptor and is not 'stdin', # close it. Avoids a Python warning about an unclosed resource. for attr in ('infile', 'outfile'): From f03b6ecae57b2efda39e742b02d4ec6aa6aeeb76 Mon Sep 17 00:00:00 2001 From: Kevin Wurster Date: Wed, 30 Oct 2024 20:32:29 -0400 Subject: [PATCH 3/8] Test against Python 3.13 --- .github/workflows/tests.yml | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index de326a4..d1b0a8e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,7 +14,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"] steps: diff --git a/pyproject.toml b/pyproject.toml index 274113a..169a268 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,7 +50,7 @@ license-files = ["LICENSE.txt"] legacy_tox_ini = """ [tox] min_version = 4.0 - env_list = py{38,39,310,311,312} + env_list = py{38,39,310,311,312,313} [testenv] deps = From 8c6033c17319dd8a35c376b56349af74d0a77dc1 Mon Sep 17 00:00:00 2001 From: Kevin Wurster Date: Wed, 30 Oct 2024 20:33:49 -0400 Subject: [PATCH 4/8] Optionally print full traceback --- docs.rst | 13 +++++++++++++ pyin.py | 2 +- tests/test_pyin.py | 17 +++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/docs.rst b/docs.rst index 9eff15a..a343f7f 100644 --- a/docs.rst +++ b/docs.rst @@ -207,6 +207,19 @@ but can be investigated: This is admittedly very hard to read, but rebuilding the command one expression at a time should reveal what is happening. +Environment Variables +--------------------- + +``PYIN_FULL_TRACEBACK`` +^^^^^^^^^^^^^^^^^^^^^^^ + +``$ pyin`` carefully manages how exceptions are raised and presented to the +user to differentiate between problems with expressions, and problems with +``$ pyin`` itself. In some cases it is helpful to get a full traceback. + +The presence of this variable in the environment enables the feature regardless +of its value. + Directives ========== diff --git a/pyin.py b/pyin.py index 6d19c9e..0c7352d 100644 --- a/pyin.py +++ b/pyin.py @@ -1332,7 +1332,7 @@ def _cli_entrypoint(rawargs=None): # A 'RuntimeError()' indicates a problem that should have been caught # during testing. We want a full traceback in these cases. - if 'PYIN_FULL_TB' in os.environ or isinstance(e, RuntimeError): + if 'PYIN_FULL_TRACEBACK' in os.environ or isinstance(e, RuntimeError): message = ''.join(traceback.format_exc()).rstrip() else: message = f"ERROR: {str(e)}" diff --git a/tests/test_pyin.py b/tests/test_pyin.py index 34738c2..43c02ea 100644 --- a/tests/test_pyin.py +++ b/tests/test_pyin.py @@ -11,6 +11,7 @@ import sys import textwrap import time +from unittest import mock import pytest @@ -323,3 +324,19 @@ def test_setup_syntax_error(runner): assert result.exit_code == 1 assert not result.output assert result.err == expected + os.linesep + + +@mock.patch.dict(os.environ, {'PYIN_FULL_TRACEBACK': ''}) +def test_PYIN_FULL_TRACEBACK(runner): + + """Test the ``PYIN_FULL_TRACEBACK`` environment variable.""" + + result = runner.invoke(_cli_entrypoint, [ + '--gen', 'range(3)', + 'i + "TypeError"' + ]) + + assert result.exit_code == 1 + assert not result.output + assert len(result.err.splitlines()) > 10 + assert 'supported operand' in result.err From 3b4cb02a0d92da660ce962bfd95c183a39c33b7d Mon Sep 17 00:00:00 2001 From: Kevin Wurster Date: Thu, 31 Oct 2024 08:44:48 -0400 Subject: [PATCH 5/8] Better CLI type metavars --- pyin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyin.py b/pyin.py index 0c7352d..daa5df7 100644 --- a/pyin.py +++ b/pyin.py @@ -1102,7 +1102,7 @@ def argparse_parser(): ) aparser.add_argument( '--linesep', - metavar='string', + metavar='STR', default=os.linesep, help=f"Write this after every line. Defaults to: {repr(os.linesep)}." ) @@ -1113,7 +1113,7 @@ def argparse_parser(): ) aparser.add_argument( '--variable', - metavar='EXPR', + metavar='STR', type=_type_variable, default=_DEFAULT_VARIABLE, help="Place each input item in this variable when evaluating" From 3dca42398e9eae8e46d07f3b2b036188a80eaa11 Mon Sep 17 00:00:00 2001 From: Kevin Wurster Date: Thu, 31 Oct 2024 08:46:01 -0400 Subject: [PATCH 6/8] Detect empty expressions An empty string or a string that is entirely white space is not a valid expression. Note that this does not catch all cases, but it does surface a more helpful error in most cases. --- pyin.py | 34 +++++++++++++++++++++++++++------- tests/test_pyin.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/pyin.py b/pyin.py index daa5df7..1caae67 100644 --- a/pyin.py +++ b/pyin.py @@ -148,18 +148,16 @@ def compile( tokens = list(expressions) del expressions - # Check for empty strings in expressions. A check with 'all(tokens)' may - # be sufficient, but could be confused by '__bool__()'. - if not all(len(t) for t in tokens): - raise SyntaxError( - f"one or more expression is an empty string:" - f" {' '.join(map(repr, tokens))}") - while tokens: # Get a directive directive = tokens.pop(0) + if directive == '' or directive.isspace(): + raise SyntaxError( + f'expression is white space or empty: {repr(directive)}' + ) + # If it is not actually a directive just assume it is a Python # expression that should be evaluated. Stick the token back in the # queue so that it can be evaluated as an argument - makes the rest @@ -1055,6 +1053,26 @@ def _type_gen(value): raise argparse.ArgumentTypeError( 'cannot combine with piping data to stdin') + return _type_expression(value) + + +def _type_expression(value): + + """Validate a Python expression argument. + + Not comprehensive. Ultimately compiling the expression to a code object + is the only method for ensuring compliance. + """ + + if value.isspace(): + raise argparse.ArgumentTypeError( + 'expression is entirely white space' + ) + elif value == '': + raise argparse.ArgumentTypeError( + 'empty expression' + ) + return value @@ -1108,6 +1126,7 @@ def argparse_parser(): ) aparser.add_argument( '-s', '--setup', action='append', metavar='EXPR', + type=_type_expression, help="Execute one or more Python statements to pre-initialize objects," " import objects with new names, etc." ) @@ -1131,6 +1150,7 @@ def argparse_parser(): aparser.add_argument( 'expressions', metavar='EXPR', + type=_type_expression, nargs='*', help='Python expression.' ) diff --git a/tests/test_pyin.py b/tests/test_pyin.py index 43c02ea..a3e383d 100644 --- a/tests/test_pyin.py +++ b/tests/test_pyin.py @@ -340,3 +340,34 @@ def test_PYIN_FULL_TRACEBACK(runner): assert not result.output assert len(result.err.splitlines()) > 10 assert 'supported operand' in result.err + + +@pytest.mark.parametrize('expr, message', [ + ('', '{tag}: empty expression'), + (' ', '{tag}: expression is entirely white space') +]) +def test_expressions_white_space(expr, message, runner): + + """Ensure empty expressions cannot be passed to ``$ pyin``.""" + + ########################################################################### + # Test expressions + + result = runner.invoke(_cli_entrypoint, [ + '--gen', 'range(3)', expr, 'i' + ]) + + assert result.exit_code == 2 + assert not result.output + assert message.format(tag='EXPR') in result.err + + ########################################################################### + # Test --gen expressions + + result = runner.invoke(_cli_entrypoint, [ + '--gen', expr + ]) + + assert result.exit_code == 2 + assert not result.output + assert message.format(tag='--gen') in result.err From d19b309c986634acdd389960f8c912866beabe55 Mon Sep 17 00:00:00 2001 From: Kevin Wurster Date: Thu, 31 Oct 2024 08:47:55 -0400 Subject: [PATCH 7/8] Better SyntaxError reporting --- pyin.py | 32 +++++++++++++++++--------------- tests/test_operations.py | 6 +++--- tests/test_pyin.py | 4 ++-- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/pyin.py b/pyin.py index 1caae67..8214286 100644 --- a/pyin.py +++ b/pyin.py @@ -524,13 +524,7 @@ def compiled_expression(self, mode): """Compile a Python expression using the builtin ``compile()``.""" - try: - return builtins.compile(self.expression, '', mode) - except SyntaxError as e: - raise SyntaxError( - f"expression {repr(self.expression)} contains a syntax error:" - f" {e.text}" - ) + return builtins.compile(self.expression, '', mode) class OpEval(OpBaseExpression, directives=('%eval', '%stream', '%exec')): @@ -1246,14 +1240,8 @@ def main( # Probably possible to use 'OpEval(%exec)' here, but not immediately # clear how to manifest the scope changes. for statement in setup: - try: - code = builtins.compile(statement, '', 'exec') - except SyntaxError as e: - raise SyntaxError( - f"setup statement contains a syntax error:" - f" {e.text.strip()}" - ) - exec(code, scope, local_scope) + code_object = builtins.compile(statement, '', 'exec') + exec(code_object, scope, local_scope) scope.update(local_scope) del local_scope @@ -1339,6 +1327,20 @@ def _cli_entrypoint(rawargs=None): try: exit_code = main(**vars(args)) + except SyntaxError as e: + + exit_code = 1 + + # Reformat the exception information to provide clarity that this is + # something the user did wrong, and not something 'pyin' did wrong. + lines = [ + f'ERROR: expression contains a syntax error: {e.msg}', + '', + f' {e.text}', + f' {" " * (e.offset - 1)}^', + ] + print(os.linesep.join(lines), file=sys.stderr) + # User interrupted with '^C' most likely, but technically this is just # a SIGINT. Somehow this shows up in the coverage report generated by # '$ pytest --cov'. No idea how that works!! diff --git a/tests/test_operations.py b/tests/test_operations.py index f362293..3cbd828 100644 --- a/tests/test_operations.py +++ b/tests/test_operations.py @@ -150,7 +150,7 @@ def test_simple_stream(directive, args, stream, expected): assert list(pyin.eval(expressions, [])) == [] -def test_Eval_syntax_error(): +def test_eval_syntax_error(): """Produce a helpful error when encountering :obj:`SyntaxError`. @@ -163,8 +163,8 @@ def test_Eval_syntax_error(): with pytest.raises(SyntaxError) as e: list(pyin.eval(expr, range(1))) - assert 'contains a syntax error' in str(e.value) - assert expr in str(e.value) + assert 'invalid syntax' in str(e.value) + assert expr == e.value.text def test_OpCSVDict(csv_with_header): diff --git a/tests/test_pyin.py b/tests/test_pyin.py index a3e383d..b293cdc 100644 --- a/tests/test_pyin.py +++ b/tests/test_pyin.py @@ -314,7 +314,6 @@ def test_setup_syntax_error(runner): """``SyntaxError`` in a setup statement.""" statement = '1 invalid syntax' - expected = f'ERROR: setup statement contains a syntax error: {statement}' result = runner.invoke(_cli_entrypoint, [ '--gen', 'range(1)', @@ -323,7 +322,8 @@ def test_setup_syntax_error(runner): assert result.exit_code == 1 assert not result.output - assert result.err == expected + os.linesep + assert 'expression contains a syntax error: invalid syntax' in result.err + assert statement in result.err @mock.patch.dict(os.environ, {'PYIN_FULL_TRACEBACK': ''}) From 04161461686e27415e8a05f30dffada6a86eed70 Mon Sep 17 00:00:00 2001 From: Kevin Wurster Date: Thu, 31 Oct 2024 08:57:50 -0400 Subject: [PATCH 8/8] Bump copyright --- LICENSE.txt | 2 +- docs.rst | 4 ++-- pyin.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/LICENSE.txt b/LICENSE.txt index e7bbee5..13b501a 100644 --- a/LICENSE.txt +++ b/LICENSE.txt @@ -1,6 +1,6 @@ New BSD License -Copyright (c) 2015-2023, Kevin D. Wurster +Copyright (c) 2015-2024, Kevin D. Wurster All rights reserved. Redistribution and use in source and binary forms, with or without diff --git a/docs.rst b/docs.rst index a343f7f..9e79386 100644 --- a/docs.rst +++ b/docs.rst @@ -160,7 +160,7 @@ A more complex example mixing directives, expressions, etc.: 'i[::2]' \ %stream '[" ".join(i) for i in s]' New License - Copyright 2015-2023, D. + Copyright 2015-2024, D. All reserved. is equivalent to the Python code: @@ -180,7 +180,7 @@ is equivalent to the Python code: ... i = i[::2] ... print(" ".join(i)) New License - Copyright 2015-2023, D. + Copyright 2015-2024, D. All reserved. Scope diff --git a/pyin.py b/pyin.py index 8214286..73a55ea 100644 --- a/pyin.py +++ b/pyin.py @@ -26,7 +26,7 @@ __license__ = ''' New BSD License -Copyright (c) 2015-2023, Kevin D. Wurster +Copyright (c) 2015-2024, Kevin D. Wurster All rights reserved. Redistribution and use in source and binary forms, with or without