Skip to content

Conversation

@PiotrCzapla
Copy link

@jph00 What do you think about centralizing it in a single helper?

The could look like this:

def view(...):
    try:
        assert (1<=s<=len(lines)), f'Invalid start line {s}'
        assert e==-1 or s<=e<= len(lines), f'Invalid end line {e}'
        ...
    except: return explain_exc("viewing")

I’m leaning towards an explicit try/except with a helper func, rather than a decorator, as I worry a decorator might suggest some "magic" to convert a function to a tool.

Re. assert, I find them easier to reason about or skip than if statements, especially with complex logic. But they can be disabled with -O. If we would like to avoid this edge case what would you say about simple ensure(b, m) helper that raise ValueError Like:

def view(...):
    try:
        ensure(1 <= s <= len(lines), f'Invalid start line {s}')
        ensure(e == -1 or s <= e <= len(lines), f'Invalid end line {e}')
        ...
    except: return explain_exc("viewing")

Regarding the explain_exc it could look like:

def explain_exception(task=''):
    try: raise sys.exc_info()[1]
    except AssertionError as e: return f"Error: {e}"
    except Exception as e: return f"Error {task}: {repr(e)}"

or using python matching

def explain_exception(task=''):
    match sys.exc_info()[1]:
        case AssertionError() as e: return f"Error: {e}"
        case e: return f"Error {task}: {repr(e)}"

I've implemented rerise as it is easier to catch multiple exceptions

Here is how the current error handling looks like
def run_cmd
    if disallow_re ...: return 'Error: args disallowed'
    if allow_re ...: return 'Error: args not allowed'
    try: ... except Exception as e: return f'Error running cmd: {str(e)}'

def rg / def sed
    uses run_cmd

def view
    if not (1<=s<=len(lines)): return f'Error: Invalid start line {s}'
    if e!=-1 and not (s<=e<= len(lines)): return f'Error: Invalid end line {e}'
    except Exception as e: return f'Error viewing: {str(e)}'

def create
    if p.exists() and not overwrite: return f'Error: File already exists: {p}'
    except Exception as e: return f'Error creating file: {str(e)}'

def insert
    if not (0 <= insert_line <= len(content)): return f'Error: Invalid line number {insert_line}'
    except Exception as e: return f'Error inserting text: {str(e)}'

def str_replace
    if count == 0: return 'Error: Text not found in file'
    if count > 1: return f'Error: Multiple matches found ({count})'
    except Exception as e: return f'Error replacing text: {str(e)}'

def strs_replace
    uses str_replace

def replace_lines
    except Exception as e: return f'Error replacing lines: {str(e)}'

def move_lines
    assert 1 <= start_line <= end_line <= len(lines), f"Invalid range {start_line}-{end_line}"
    assert 1 <= dest_line <= len(lines) + 1, f"Invalid destination {dest_line}"
    assert not(start_line <= dest_line <= end_line + 1), "Destination within source range"
    except Exception as e: return f'Error: {str(e)}'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant