Allow further options for linters

[mwermelinger]

The current approach with IPython.core.magic_argument is restrictive because it requires duplicating all the linter's options, otherwise there's a UsageError. For example, it's not currently possible to write %allowed -m on in case pytype is installed, or %allowed -u 10 on if one wants to check the current notebook only against units 1–10. (This is necessary for the M269 TMAs.)

A more flexible approach is to simply write %<linter> on ... and pass ... to that linter. However, how to make sure that the default configuration (like --ignore for ruff or -c for allowed) is overwritten if the user specifies it?

Two approaches I can think of:

• Add an optional positional string argument for the options to pass to the linter, e.g. %allowed -c tm112.json "-m -u 10" on. It's a bit clunky to have some options in a string and others not (those we have defaults for). Maybe there's a way to not require a string, and just have zero or more extra arguments for the command?
• Do our own argument processing to permit %linter [on [...]|off], by splitting the arguments string around spaces and checking if one of the options overrides a default option.

The first option is less work, especially if we can do it without needing double-quotes for the extra options.

[densnow]

If I am interpreting the docs correctly, then we can add nargs='*' to an @argument and gather args that follow in a list. The following example is given:

>>>parser = argparse.ArgumentParser()
>>>parser.add_argument('--foo', nargs='*')
>>>parser.add_argument('--bar', nargs='*')
>>>parser.add_argument('baz', nargs='*')
>>>parser.parse_args('a b --foo x y --bar 1 2'.split())
Namespace(bar=['1', '2'], baz=['a', 'b'], foo=['x', 'y'])

We could have some kind of option like --passthrough --cmd-args or --raw-args that precedes the args we want to pass to the underlying linter.

[mwermelinger]

I prefer not to require the user to type more than needed. And since the args have hyphens themselves, will they be interpreted as unknown options, ie does -a b add -a and b to baz, or just b, or neither, with an error that -a is an unknown exception? I guess we'd have to experiment a bit...

[densnow]

Yes you are right, the args with dashes would be interpreted as unknown options when using parse_argstring().

Another option is to ditch the @magic_arguments and @argument, and instead use parse_known_args() from the regular argparse library.

@register_line_magic
def my_magic(line: str) -> None:
    parser = argparse.ArgumentParser()
    parser.add_argument('--config')
    parser.add_argument(
        "status",
        choices=["on", "off"],
        type=str.lower,
        help="Activate or deactivate the linter. If omitted, show the current status.",
        nargs="?",
        default=None,
    )
    known, unknown = parser.parse_known_args(line.split())
    print(f"Known args: {known}")
    print(f"Unknown args: {unknown}") 

%my_magic on --config m269.json -b another_arg --unknown -s

Known args: Namespace(config='m269.json', status='on')
Unknown args: ['-b', 'another_arg', '--unknown', '-s']

If we are going to pass unknown args on to the linters, then we probably need to allow error messages related to abnormal termination to be shown. At present show_errors(), show_ruff_json() and show_pythpe_errors() are preventing that (as mentioned in #8).

[mwermelinger]

I've pushed the changes to allow for extra command options, and updated the docs. For some reason, argparse can separate known and unknown arguments only if on is the first argument. So, %allowed on -u 10 works but %allowed -u 10 on doesn't. I changed the docstrings for on to appear first.

Over to you to catch any command option errors. Thanks.

[densnow]

It seems argparse deems any arg added without a dash at the start of its name to be positional. This must be slightly different from the behaviour of the @magic_arguments

I've pushed the changes to handle better the error messages from abnormal terminations of the linters. This mostly involves checking CompletedProcess.stderr for ruff and pytype, but allowed prints errors to both stdout and stderr, so this has been taken into account.

It might be best for allowed to print messages from abnormal termination only to stderr in the future to follow convention.

[densnow]

It looks like the show_ruff_json function was filtering out a deprecation warning related to the structure of the ruff configuration in the pyproject.toml.

Also, this means with the current changes, if there is a warning, then style violations will not be printed. Do we want to let a warning stop the printing of style violations? It can be changed relatively easily to print both warnings and style violations.

[mwermelinger]

Reopening this issue to allow @densnow to show both stderr and stdout, in case stderr has warnings. The message "{checker} didn't check code" must be changed.