Add style tool to fix fish file formatting (#39155)

This commit is contained in:
Adam J. Stewart 2023-08-28 22:16:44 -05:00 committed by GitHub
parent d9d1eb24f9
commit 70c71e8f93
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 517 additions and 493 deletions

View file

@ -37,6 +37,7 @@ def spack_dev_requirements(cls) -> List[str]:
mypy_root_spec(), mypy_root_spec(),
black_root_spec(), black_root_spec(),
flake8_root_spec(), flake8_root_spec(),
fish_root_spec(),
pytest_root_spec(), pytest_root_spec(),
] ]
@ -179,6 +180,12 @@ def flake8_root_spec() -> str:
return _root_spec("py-flake8@3.8.2:") return _root_spec("py-flake8@3.8.2:")
def fish_root_spec() -> str:
"""Return the root spec used to bootstrap fish"""
# fish 3.2.0 introduces the `--check` flag to `fish_indent`
return _root_spec("fish@3.2:")
def pytest_root_spec() -> str: def pytest_root_spec() -> str:
"""Return the root spec used to bootstrap flake8""" """Return the root spec used to bootstrap flake8"""
return _root_spec("py-pytest@6.2.4:") return _root_spec("py-pytest@6.2.4:")

View file

@ -36,7 +36,7 @@ def grouper(iterable, n, fillvalue=None):
#: double-check the results of other tools (if, e.g., --fix was provided) #: double-check the results of other tools (if, e.g., --fix was provided)
#: The list maps an executable name to a method to ensure the tool is #: The list maps an executable name to a method to ensure the tool is
#: bootstrapped or present in the environment. #: bootstrapped or present in the environment.
tool_names = ["isort", "black", "flake8", "mypy"] tool_names = ["isort", "black", "flake8", "mypy", "fish_indent"]
#: tools we run in spack style #: tools we run in spack style
tools = {} tools = {}
@ -61,12 +61,13 @@ def is_package(f):
#: decorator for adding tools to the list #: decorator for adding tools to the list
class tool: class tool:
def __init__(self, name, required=False): def __init__(self, name, required=False, suffix=".py"):
self.name = name self.name = name
self.required = required self.required = required
self.suffix = suffix
def __call__(self, fun): def __call__(self, fun):
tools[self.name] = (fun, self.required) tools[self.name] = (fun, self.required, self.suffix)
return fun return fun
@ -121,12 +122,8 @@ def changed_files(base="develop", untracked=True, all_files=False, root=None):
files = git(*arg_list, output=str).split("\n") files = git(*arg_list, output=str).split("\n")
for f in files: for f in files:
# Ignore non-Python files
if not (f.endswith(".py") or f == "bin/spack"):
continue
# Ignore files in the exclude locations # Ignore files in the exclude locations
if any(os.path.realpath(f).startswith(e) for e in excludes): if not f or any(os.path.realpath(f).startswith(e) for e in excludes):
continue continue
changed.add(f) changed.add(f)
@ -352,6 +349,22 @@ def run_black(black_cmd, file_list, args):
return returncode return returncode
@tool("fish_indent", suffix=".fish")
def run_fish_indent(fish_indent_cmd, file_list, args):
if args.fix:
fish_indent_args = ["--write"]
else:
fish_indent_args = ["--check"]
output = fish_indent_cmd(*fish_indent_args, *file_list, fail_on_error=False, output=str)
returncode = fish_indent_cmd.returncode
rewrite_and_print_output(output, args)
print_tool_result("fish_indent", returncode)
return returncode
def validate_toolset(arg_value): def validate_toolset(arg_value):
"""Validate --tool and --skip arguments (sets of optionally comma-separated tools).""" """Validate --tool and --skip arguments (sets of optionally comma-separated tools)."""
tools = set(",".join(arg_value).split(",")) # allow args like 'isort,flake8' tools = set(",".join(arg_value).split(",")) # allow args like 'isort,flake8'
@ -417,9 +430,11 @@ def prefix_relative(path):
print_style_header(file_list, args, tools_to_run) print_style_header(file_list, args, tools_to_run)
for tool_name in tools_to_run: for tool_name in tools_to_run:
run_function, required = tools[tool_name] run_function, required, suffix = tools[tool_name]
print_tool_header(tool_name) print_tool_header(tool_name)
return_code |= run_function(which(tool_name), file_list, args) file_subset = [f for f in file_list if f.endswith(suffix)]
if file_subset:
return_code |= run_function(which(tool_name), file_subset, args)
if return_code == 0: if return_code == 0:
tty.msg(color.colorize("@*{spack style checks were clean}")) tty.msg(color.colorize("@*{spack style checks were clean}"))

View file

@ -56,7 +56,7 @@ def flake8_package_with_errors(scope="function"):
"""A flake8 package with errors.""" """A flake8 package with errors."""
repo = spack.repo.Repo(spack.paths.mock_packages_path) repo = spack.repo.Repo(spack.paths.mock_packages_path)
filename = repo.filename_for_package_name("flake8") filename = repo.filename_for_package_name("flake8")
tmp = filename + ".tmp" tmp = filename.replace("package.py", "package2.py")
shutil.copy(filename, tmp) shutil.copy(filename, tmp)
package = FileFilter(tmp) package = FileFilter(tmp)
@ -64,11 +64,12 @@ def flake8_package_with_errors(scope="function"):
# this is a black error (quote style and spacing before/after operator) # this is a black error (quote style and spacing before/after operator)
package.filter('state = "unmodified"', "state = 'modified'", string=True) package.filter('state = "unmodified"', "state = 'modified'", string=True)
# this is an isort error (orderign) and a flake8 error (unused import) # this is an isort error (ordering) and a flake8 error (unused import)
package.filter( package.filter(
"from spack.package import *", "from spack.package import *\nimport os", string=True "from spack.package import *", "from spack.package import *\nimport os", string=True
) )
yield tmp yield tmp
os.remove(tmp)
def test_changed_files_from_git_rev_base(git, tmpdir, capfd): def test_changed_files_from_git_rev_base(git, tmpdir, capfd):
@ -213,6 +214,7 @@ def test_fix_style(external_style_root):
@pytest.mark.skipif(not which("isort"), reason="isort is not installed.") @pytest.mark.skipif(not which("isort"), reason="isort is not installed.")
@pytest.mark.skipif(not which("mypy"), reason="mypy is not installed.") @pytest.mark.skipif(not which("mypy"), reason="mypy is not installed.")
@pytest.mark.skipif(not which("black"), reason="black is not installed.") @pytest.mark.skipif(not which("black"), reason="black is not installed.")
@pytest.mark.skipif(not which("fish_indent"), reason="fish is not installed.")
def test_external_root(external_style_root, capfd): def test_external_root(external_style_root, capfd):
"""Ensure we can run in a separate root directory w/o configuration files.""" """Ensure we can run in a separate root directory w/o configuration files."""
tmpdir, py_file = external_style_root tmpdir, py_file = external_style_root
@ -285,5 +287,5 @@ def test_style_with_black(flake8_package_with_errors):
def test_skip_tools(): def test_skip_tools():
output = style("--skip", "isort,mypy,black,flake8") output = style("--skip", "isort,mypy,black,flake8,fish_indent")
assert "Nothing to run" in output assert "Nothing to run" in output

View file

@ -66,51 +66,51 @@ end
function spack -d "wrapper for the `spack` command" function spack -d "wrapper for the `spack` command"
# #
# DEFINE SUPPORT FUNCTIONS HERE # DEFINE SUPPORT FUNCTIONS HERE
# #
# #
# ALLOCATE_SP_SHARED, and DELETE_SP_SHARED allocate (and delete) temporary # ALLOCATE_SP_SHARED, and DELETE_SP_SHARED allocate (and delete) temporary
# global variables # global variables
# #
function allocate_sp_shared -d "allocate shared (global variables)" function allocate_sp_shared -d "allocate shared (global variables)"
set -gx __sp_remaining_args set -gx __sp_remaining_args
set -gx __sp_subcommand_args set -gx __sp_subcommand_args
set -gx __sp_module_args set -gx __sp_module_args
set -gx __sp_stat set -gx __sp_stat
set -gx __sp_stdout set -gx __sp_stdout
set -gx __sp_stderr set -gx __sp_stderr
end end
function delete_sp_shared -d "deallocate shared (global variables)" function delete_sp_shared -d "deallocate shared (global variables)"
set -e __sp_remaining_args set -e __sp_remaining_args
set -e __sp_subcommand_args set -e __sp_subcommand_args
set -e __sp_module_args set -e __sp_module_args
set -e __sp_stat set -e __sp_stat
set -e __sp_stdout set -e __sp_stdout
set -e __sp_stderr set -e __sp_stderr
end end
# #
# STREAM_ARGS and SHIFT_ARGS: helper functions manipulating the `argv` array: # STREAM_ARGS and SHIFT_ARGS: helper functions manipulating the `argv` array:
# -> STREAM_ARGS: echos the `argv` array element-by-element # -> STREAM_ARGS: echos the `argv` array element-by-element
# -> SHIFT_ARGS: echos the `argv` array element-by-element starting with the # -> SHIFT_ARGS: echos the `argv` array element-by-element starting with the
# second element. If `argv` has only one element, echo the # second element. If `argv` has only one element, echo the
# empty string `""`. # empty string `""`.
# NOTE: while `stream_args` is not strictly necessary, it adds a nice symmetry # NOTE: while `stream_args` is not strictly necessary, it adds a nice symmetry
# to `shift_args` # to `shift_args`
# #
function stream_args -d "echos args as a stream" function stream_args -d "echos args as a stream"
# return the elements of `$argv` as an array # return the elements of `$argv` as an array
# -> since we want to be able to call it as part of `set x (shift_args # -> since we want to be able to call it as part of `set x (shift_args
# $x)`, we return these one-at-a-time using echo... this means that the # $x)`, we return these one-at-a-time using echo... this means that the
@ -118,10 +118,10 @@ function stream_args -d "echos args as a stream"
for elt in $argv for elt in $argv
echo $elt echo $elt
end end
end end
function shift_args -d "simulates bash shift" function shift_args -d "simulates bash shift"
# #
# Returns argv[2..-1] (as an array) # Returns argv[2..-1] (as an array)
# -> if argv has only 1 element, then returns the empty string. This # -> if argv has only 1 element, then returns the empty string. This
@ -142,21 +142,21 @@ function shift_args -d "simulates bash shift"
end end
end end
end end
# #
# CAPTURE_ALL: helper function used to capture stdout, stderr, and status # CAPTURE_ALL: helper function used to capture stdout, stderr, and status
# -> CAPTURE_ALL: there is a bug in fish, that prevents stderr re-capture # -> CAPTURE_ALL: there is a bug in fish, that prevents stderr re-capture
# from nested command substitution: # from nested command substitution:
# https://github.com/fish-shell/fish-shell/issues/6459 # https://github.com/fish-shell/fish-shell/issues/6459
# #
function capture_all function capture_all
begin; begin
begin; begin
eval $argv[1] eval $argv[1]
set $argv[2] $status # read sets the `status` flag => capture here set $argv[2] $status # read sets the `status` flag => capture here
end 2>| read -z __err end 2>| read -z __err
@ -167,18 +167,18 @@ function capture_all
set $argv[4] (echo $__err | string split \n) set $argv[4] (echo $__err | string split \n)
return 0 return 0
end end
# #
# GET_SP_FLAGS, and GET_MOD_ARGS: support functions for extracting arguments and # GET_SP_FLAGS, and GET_MOD_ARGS: support functions for extracting arguments and
# flags. Note bash's `shift` operation is simulated by the `__sp_remaining_args` # flags. Note bash's `shift` operation is simulated by the `__sp_remaining_args`
# array which is roughly equivalent to `$@` in bash. # array which is roughly equivalent to `$@` in bash.
# #
function get_sp_flags -d "return leading flags" function get_sp_flags -d "return leading flags"
# #
# Accumulate initial flags for main spack command. NOTE: Sets the external # Accumulate initial flags for main spack command. NOTE: Sets the external
# array: `__sp_remaining_args` containing all unprocessed arguments. # array: `__sp_remaining_args` containing all unprocessed arguments.
@ -213,16 +213,16 @@ function get_sp_flags -d "return leading flags"
# if all elements in `argv` are matched, make sure that `__sp_remaining_args` # if all elements in `argv` are matched, make sure that `__sp_remaining_args`
# is deleted (this might be overkill...). # is deleted (this might be overkill...).
set -e __sp_remaining_args set -e __sp_remaining_args
end end
# #
# CHECK_SP_FLAGS, CONTAINS_HELP_FLAGS, CHECK_ENV_ACTIVATE_FLAGS, and # CHECK_SP_FLAGS, CONTAINS_HELP_FLAGS, CHECK_ENV_ACTIVATE_FLAGS, and
# CHECK_ENV_DEACTIVATE_FLAGS: support functions for checking arguments and flags. # CHECK_ENV_DEACTIVATE_FLAGS: support functions for checking arguments and flags.
# #
function check_sp_flags -d "check spack flags for h/V flags" function check_sp_flags -d "check spack flags for h/V flags"
# #
# Check if inputs contain h or V flags. # Check if inputs contain h or V flags.
# #
@ -242,11 +242,11 @@ function check_sp_flags -d "check spack flags for h/V flags"
end end
return 1 return 1
end end
function match_flag -d "checks all combinations of flags ocurring inside of a string" function match_flag -d "checks all combinations of flags ocurring inside of a string"
# Remove leading and trailing spaces -- but we need to insert a "guard" (x) # Remove leading and trailing spaces -- but we need to insert a "guard" (x)
# so that eg. `string trim -h` doesn't trigger the help string for `string trim` # so that eg. `string trim -h` doesn't trigger the help string for `string trim`
@ -279,11 +279,11 @@ function match_flag -d "checks all combinations of flags ocurring inside of a st
return 1 return 1
end end
function check_env_activate_flags -d "check spack env subcommand flags for -h, --sh, --csh, or --fish" function check_env_activate_flags -d "check spack env subcommand flags for -h, --sh, --csh, or --fish"
# #
# Check if inputs contain -h/--help, --sh, --csh, or --fish # Check if inputs contain -h/--help, --sh, --csh, or --fish
# #
@ -296,42 +296,42 @@ function check_env_activate_flags -d "check spack env subcommand flags for -h, -
if test -n "$_a" if test -n "$_a"
# looks for a single `-h` # looks for a single `-h`
if match_flag $_a "-h" if match_flag $_a -h
return 0 return 0
end end
# looks for a single `--help` # looks for a single `--help`
if match_flag $_a "--help" if match_flag $_a --help
return 0 return 0
end end
# looks for a single `--sh` # looks for a single `--sh`
if match_flag $_a "--sh" if match_flag $_a --sh
return 0 return 0
end end
# looks for a single `--csh` # looks for a single `--csh`
if match_flag $_a "--csh" if match_flag $_a --csh
return 0 return 0
end end
# looks for a single `--fish` # looks for a single `--fish`
if match_flag $_a "--fish" if match_flag $_a --fish
return 0 return 0
end end
# looks for a single `--list` # looks for a single `--list`
if match_flag $_a "--list" if match_flag $_a --list
return 0 return 0
end end
end end
return 1 return 1
end end
function check_env_deactivate_flags -d "check spack env subcommand flags for --sh, --csh, or --fish" function check_env_deactivate_flags -d "check spack env subcommand flags for --sh, --csh, or --fish"
# #
# Check if inputs contain --sh, --csh, or --fish # Check if inputs contain --sh, --csh, or --fish
# #
@ -344,34 +344,34 @@ function check_env_deactivate_flags -d "check spack env subcommand flags for --s
if test -n "$_a" if test -n "$_a"
# looks for a single `--sh` # looks for a single `--sh`
if match_flag $_a "--sh" if match_flag $_a --sh
return 0 return 0
end end
# looks for a single `--csh` # looks for a single `--csh`
if match_flag $_a "--csh" if match_flag $_a --csh
return 0 return 0
end end
# looks for a single `--fish` # looks for a single `--fish`
if match_flag $_a "--fish" if match_flag $_a --fish
return 0 return 0
end end
end end
return 1 return 1
end end
# #
# SPACK RUNNER function, this does all the work! # SPACK RUNNER function, this does all the work!
# #
function spack_runner -d "Runner function for the `spack` wrapper" function spack_runner -d "Runner function for the `spack` wrapper"
# Store DYLD_* variables from spack shell function # Store DYLD_* variables from spack shell function
# This is necessary because MacOS System Integrity Protection clears # This is necessary because MacOS System Integrity Protection clears
# variables that affect dyld on process start. # variables that affect dyld on process start.
@ -424,7 +424,7 @@ function spack_runner -d "Runner function for the `spack` wrapper"
# further needs to be done. Otherwise, test the location referring the # further needs to be done. Otherwise, test the location referring the
# subcommand and cd there (if it exists). # subcommand and cd there (if it exists).
case "cd" case cd
set -l sp_arg "" set -l sp_arg ""
@ -435,7 +435,7 @@ function spack_runner -d "Runner function for the `spack` wrapper"
end end
# Notes: [2] (cf. EOF) # Notes: [2] (cf. EOF)
if test "x$sp_arg" = "x-h"; or test "x$sp_arg" = "x--help" if test "x$sp_arg" = x-h; or test "x$sp_arg" = x--help
# nothing more needs to be done for `-h` or `--help` # nothing more needs to be done for `-h` or `--help`
command spack cd -h command spack cd -h
return return
@ -459,7 +459,7 @@ function spack_runner -d "Runner function for the `spack` wrapper"
# varibles. These commands are then run by fish (using the `capture_all` # varibles. These commands are then run by fish (using the `capture_all`
# function, instead of a command substitution). # function, instead of a command substitution).
case "env" case env
set -l sp_arg "" set -l sp_arg ""
@ -470,13 +470,13 @@ function spack_runner -d "Runner function for the `spack` wrapper"
end end
# Notes: [2] (cf. EOF) # Notes: [2] (cf. EOF)
if test "x$sp_arg" = "x-h"; or test "x$sp_arg" = "x--help" if test "x$sp_arg" = x-h; or test "x$sp_arg" = x--help
# nothing more needs to be done for `-h` or `--help` # nothing more needs to be done for `-h` or `--help`
command spack env -h command spack env -h
return return
else else
switch $sp_arg switch $sp_arg
case "activate" case activate
set -l _a (stream_args $__sp_remaining_args) set -l _a (stream_args $__sp_remaining_args)
if check_env_activate_flags $_a if check_env_activate_flags $_a
@ -494,7 +494,7 @@ function spack_runner -d "Runner function for the `spack` wrapper"
return $__sp_stat return $__sp_stat
end end
case "deactivate" case deactivate
set -l _a (stream_args $__sp_remaining_args) set -l _a (stream_args $__sp_remaining_args)
if check_env_deactivate_flags $_a if check_env_deactivate_flags $_a
@ -543,7 +543,7 @@ function spack_runner -d "Runner function for the `spack` wrapper"
# to deal with the substituting latest version numbers to the module # to deal with the substituting latest version numbers to the module
# command. # command.
case "load" or "unload" case load or unload
set -l _a (stream_args $__sp_remaining_args) set -l _a (stream_args $__sp_remaining_args)
@ -571,43 +571,43 @@ function spack_runner -d "Runner function for the `spack` wrapper"
command spack $argv command spack $argv
return return
end end
end end
# #
# RUN SPACK_RUNNER HERE # RUN SPACK_RUNNER HERE
# #
# #
# Allocate temporary global variables used for return extra arguments from # Allocate temporary global variables used for return extra arguments from
# functions. NOTE: remember to call delete_sp_shared whenever returning from # functions. NOTE: remember to call delete_sp_shared whenever returning from
# this function. # this function.
# #
allocate_sp_shared allocate_sp_shared
# #
# Run spack command using the spack_runner. # Run spack command using the spack_runner.
# #
spack_runner $argv spack_runner $argv
# Capture state of spack_runner (returned below) # Capture state of spack_runner (returned below)
set -l stat $status set -l stat $status
# #
# Delete temprary global variabels allocated in `allocated_sp_shared`. # Delete temprary global variabels allocated in `allocated_sp_shared`.
# #
delete_sp_shared delete_sp_shared
return $stat return $stat
end end
@ -711,8 +711,8 @@ set -xg SPACK_ROOT $sp_prefix
# #
# No need to determine which shell is being used (obviously it's fish) # No need to determine which shell is being used (obviously it's fish)
# #
set -xg SPACK_SHELL "fish" set -xg SPACK_SHELL fish
set -xg _sp_shell "fish" set -xg _sp_shell fish
@ -721,9 +721,9 @@ if test -z "$SPACK_SKIP_MODULES"
# #
# Check whether we need environment-variables (module) <= `use` is not available # Check whether we need environment-variables (module) <= `use` is not available
# #
set -l need_module "no" set -l need_module no
if not functions -q use; and not functions -q module if not functions -q use; and not functions -q module
set need_module "yes" set need_module yes
end end
@ -742,7 +742,7 @@ if test -z "$SPACK_SKIP_MODULES"
end end
if test "$need_module" = "yes" if test "$need_module" = yes
set -l sp_shell_vars (command spack --print-shell-vars sh,modules) set -l sp_shell_vars (command spack --print-shell-vars sh,modules)
for sp_var_expr in $sp_shell_vars for sp_var_expr in $sp_shell_vars
@ -750,7 +750,7 @@ if test -z "$SPACK_SKIP_MODULES"
end end
# _sp_module_prefix is set by spack --print-sh-vars # _sp_module_prefix is set by spack --print-sh-vars
if test "$_sp_module_prefix" != "not_installed" if test "$_sp_module_prefix" != not_installed
set -xg MODULE_PREFIX $_sp_module_prefix set -xg MODULE_PREFIX $_sp_module_prefix
spack_pathadd PATH "$MODULE_PREFIX/bin" spack_pathadd PATH "$MODULE_PREFIX/bin"
end end
@ -765,7 +765,7 @@ if test -z "$SPACK_SKIP_MODULES"
end end
if test "$need_module" = "yes" if test "$need_module" = yes
function module -d "wrapper for the `module` command to point at Spack's modules instance" --inherit-variable MODULE_PREFIX function module -d "wrapper for the `module` command to point at Spack's modules instance" --inherit-variable MODULE_PREFIX
eval $MODULE_PREFIX/bin/modulecmd $SPACK_SHELL $argv eval $MODULE_PREFIX/bin/modulecmd $SPACK_SHELL $argv
end end

View file

@ -2643,9 +2643,9 @@ complete -c spack -n '__fish_spack_using_command style' -s f -l fix -d 'format a
complete -c spack -n '__fish_spack_using_command style' -l root -r -f -a root complete -c spack -n '__fish_spack_using_command style' -l root -r -f -a root
complete -c spack -n '__fish_spack_using_command style' -l root -r -d 'style check a different spack instance' complete -c spack -n '__fish_spack_using_command style' -l root -r -d 'style check a different spack instance'
complete -c spack -n '__fish_spack_using_command style' -s t -l tool -r -f -a tool complete -c spack -n '__fish_spack_using_command style' -s t -l tool -r -f -a tool
complete -c spack -n '__fish_spack_using_command style' -s t -l tool -r -d 'specify which tools to run (default: isort,black,flake8,mypy)' complete -c spack -n '__fish_spack_using_command style' -s t -l tool -r -d 'specify which tools to run (default: isort,black,flake8,mypy,fish_indent)'
complete -c spack -n '__fish_spack_using_command style' -s s -l skip -r -f -a skip complete -c spack -n '__fish_spack_using_command style' -s s -l skip -r -f -a skip
complete -c spack -n '__fish_spack_using_command style' -s s -l skip -r -d 'specify tools to skip (choose from isort,black,flake8,mypy)' complete -c spack -n '__fish_spack_using_command style' -s s -l skip -r -d 'specify tools to skip (choose from isort,black,flake8,mypy,fish_indent)'
# spack tags # spack tags
set -g __fish_spack_optspecs_spack_tags h/help i/installed a/all set -g __fish_spack_optspecs_spack_tags h/help i/installed a/all