refactor(kyberforge): consolidate vale-wrap.sh's config parsing and subprocess spawns
Two efficiency findings from a code-review pass: - The separated (--config X) and joined (--config=X) argument branches duplicated ~20 lines of absolutize-if-relative path logic. Extracted into abs_config_value(), used by both branches; the redundant --config=/* special case falls out since the helper already passes absolute paths through unchanged. - The common single-file path spawned python3 twice per file (once for abspath resolution, once for flatten()). flatten() now optionally takes a tmpdir arg and does both in one process. The per-file loop under a directory argument is unchanged — that path wasn't flagged. agent-audit's copy is canonical; skill-audit's copy was regenerated via scripts/sync-vale-styles.sh, not hand-edited, to guarantee byte parity. No hardening (bash 3.2 compat, surrogateescape, symlink guards) touched. Verified: tests/test-vale-wrap.sh 39/39, check-vale-style-sync.sh clean, full suite 12/12.
This commit is contained in:
@@ -77,6 +77,17 @@ is_builtin_output() {
|
||||
*) false ;;
|
||||
esac
|
||||
}
|
||||
# Absolutizes a `--config` value against the caller's cwd. Shared by both
|
||||
# argument forms below — separated (`--config X`) and joined (`--config=X`)
|
||||
# — so the "already absolute vs. needs $cwd prefixed" check lives in exactly
|
||||
# one place instead of being duplicated per form.
|
||||
abs_config_value() {
|
||||
if [[ "$1" == /* ]]; then
|
||||
printf '%s' "$1"
|
||||
else
|
||||
printf '%s' "$cwd/$1"
|
||||
fi
|
||||
}
|
||||
for arg in "$@"; do
|
||||
if [[ -n "$pending_flag" ]]; then
|
||||
# Value of a separated two-argv flag. It is never a lint target, however
|
||||
@@ -85,11 +96,7 @@ for arg in "$@"; do
|
||||
case "$pending_flag" in
|
||||
--config)
|
||||
# Always a path, and required to exist.
|
||||
if [[ "$arg" == /* ]]; then
|
||||
vale_args+=("$arg")
|
||||
else
|
||||
vale_args+=("$cwd/$arg")
|
||||
fi
|
||||
vale_args+=("$(abs_config_value "$arg")")
|
||||
;;
|
||||
--output|--path)
|
||||
# See `is_builtin_output` above for why the built-in `--output` names
|
||||
@@ -117,13 +124,8 @@ for arg in "$@"; do
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
--config=/*)
|
||||
vale_args+=("$arg")
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
--config=*)
|
||||
vale_args+=("--config=$cwd/${arg#--config=}")
|
||||
vale_args+=("--config=$(abs_config_value "${arg#--config=}")")
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
@@ -204,11 +206,33 @@ abspath() {
|
||||
}
|
||||
|
||||
flatten() {
|
||||
python3 - "$1" "$2" <<'PYTHON'
|
||||
# Two call shapes: `flatten src dest` (dest already resolved and inside the
|
||||
# scratch tree — the per-markdown-file calls in the directory branch below)
|
||||
# writes straight to `dest`. `flatten src raw_dest tmpdir` (the single-file
|
||||
# branch further down) additionally resolves `raw_dest` the way a separate
|
||||
# `abspath` call used to, applies the same sandbox-escape guard, and prints
|
||||
# the resolved path — folding two python3 spawns per file into one.
|
||||
python3 - "$@" <<'PYTHON'
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
src, dest = sys.argv[1], sys.argv[2]
|
||||
src, dest_input = sys.argv[1], sys.argv[2]
|
||||
tmpdir = sys.argv[3] if len(sys.argv) > 3 else None
|
||||
|
||||
if tmpdir is None:
|
||||
dest = dest_input
|
||||
else:
|
||||
dest = os.path.abspath(dest_input)
|
||||
if not dest.startswith(tmpdir + os.sep):
|
||||
print(
|
||||
f"vale-wrap.sh: refusing to lint '{src}': its scratch copy would "
|
||||
f"land outside {tmpdir}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(2)
|
||||
os.makedirs(os.path.dirname(dest), exist_ok=True)
|
||||
|
||||
# surrogateescape keeps a non-UTF-8 file (reachable via a directory argument)
|
||||
# a byte-for-byte round trip instead of aborting the whole run on a decode error.
|
||||
with open(src, encoding='utf-8', errors='surrogateescape') as fh:
|
||||
@@ -435,6 +459,9 @@ if header_m:
|
||||
|
||||
with open(dest, 'w', encoding='utf-8', errors='surrogateescape') as fh:
|
||||
fh.write(content)
|
||||
|
||||
if tmpdir is not None:
|
||||
print(dest)
|
||||
PYTHON
|
||||
}
|
||||
|
||||
@@ -449,23 +476,23 @@ mkdir -p "$mirror"
|
||||
argv_paths=()
|
||||
for arg in ${path_args[@]+"${path_args[@]}"}; do
|
||||
if [[ "$arg" == /* ]]; then
|
||||
dest="$tmpdir$arg"
|
||||
raw_dest="$tmpdir$arg"
|
||||
else
|
||||
dest="$mirror/$arg"
|
||||
raw_dest="$mirror/$arg"
|
||||
fi
|
||||
dest="$(abspath "$dest")"
|
||||
# A path argument with enough leading `..` to climb past the mirror root would
|
||||
# write outside the scratch dir. The real filesystem clamps such a path at
|
||||
# `/`; the mirror can't, so refuse rather than scribble outside the sandbox.
|
||||
case "$dest" in
|
||||
"$tmpdir"/*) ;;
|
||||
*)
|
||||
echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
mkdir -p "$(dirname "$dest")"
|
||||
if [[ -d "$arg" ]]; then
|
||||
dest="$(abspath "$raw_dest")"
|
||||
# A path argument with enough leading `..` to climb past the mirror root would
|
||||
# write outside the scratch dir. The real filesystem clamps such a path at
|
||||
# `/`; the mirror can't, so refuse rather than scribble outside the sandbox.
|
||||
case "$dest" in
|
||||
"$tmpdir"/*) ;;
|
||||
*)
|
||||
echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
mkdir -p "$(dirname "$dest")"
|
||||
# A directory is mirrored whole — vale applies its own format filtering to
|
||||
# the tree, so any file dropped here would be silently unlinted — and then
|
||||
# every markdown file in the copy is flattened in place. `.git` is pruned:
|
||||
@@ -484,7 +511,9 @@ for arg in ${path_args[@]+"${path_args[@]}"}; do
|
||||
flatten "$md" "$md"
|
||||
done < <(find "$dest" -type f -name '*.md' -print0)
|
||||
else
|
||||
flatten "$arg" "$dest"
|
||||
# `abspath` + `flatten` folded into one python3 process — see the comment
|
||||
# atop `flatten` above.
|
||||
dest="$(flatten "$arg" "$raw_dest" "$tmpdir")"
|
||||
fi
|
||||
if [[ "$arg" == /* ]]; then
|
||||
argv_paths+=("$dest")
|
||||
|
||||
@@ -77,6 +77,17 @@ is_builtin_output() {
|
||||
*) false ;;
|
||||
esac
|
||||
}
|
||||
# Absolutizes a `--config` value against the caller's cwd. Shared by both
|
||||
# argument forms below — separated (`--config X`) and joined (`--config=X`)
|
||||
# — so the "already absolute vs. needs $cwd prefixed" check lives in exactly
|
||||
# one place instead of being duplicated per form.
|
||||
abs_config_value() {
|
||||
if [[ "$1" == /* ]]; then
|
||||
printf '%s' "$1"
|
||||
else
|
||||
printf '%s' "$cwd/$1"
|
||||
fi
|
||||
}
|
||||
for arg in "$@"; do
|
||||
if [[ -n "$pending_flag" ]]; then
|
||||
# Value of a separated two-argv flag. It is never a lint target, however
|
||||
@@ -85,11 +96,7 @@ for arg in "$@"; do
|
||||
case "$pending_flag" in
|
||||
--config)
|
||||
# Always a path, and required to exist.
|
||||
if [[ "$arg" == /* ]]; then
|
||||
vale_args+=("$arg")
|
||||
else
|
||||
vale_args+=("$cwd/$arg")
|
||||
fi
|
||||
vale_args+=("$(abs_config_value "$arg")")
|
||||
;;
|
||||
--output|--path)
|
||||
# See `is_builtin_output` above for why the built-in `--output` names
|
||||
@@ -117,13 +124,8 @@ for arg in "$@"; do
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
--config=/*)
|
||||
vale_args+=("$arg")
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
--config=*)
|
||||
vale_args+=("--config=$cwd/${arg#--config=}")
|
||||
vale_args+=("--config=$(abs_config_value "${arg#--config=}")")
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
@@ -204,11 +206,33 @@ abspath() {
|
||||
}
|
||||
|
||||
flatten() {
|
||||
python3 - "$1" "$2" <<'PYTHON'
|
||||
# Two call shapes: `flatten src dest` (dest already resolved and inside the
|
||||
# scratch tree — the per-markdown-file calls in the directory branch below)
|
||||
# writes straight to `dest`. `flatten src raw_dest tmpdir` (the single-file
|
||||
# branch further down) additionally resolves `raw_dest` the way a separate
|
||||
# `abspath` call used to, applies the same sandbox-escape guard, and prints
|
||||
# the resolved path — folding two python3 spawns per file into one.
|
||||
python3 - "$@" <<'PYTHON'
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
src, dest = sys.argv[1], sys.argv[2]
|
||||
src, dest_input = sys.argv[1], sys.argv[2]
|
||||
tmpdir = sys.argv[3] if len(sys.argv) > 3 else None
|
||||
|
||||
if tmpdir is None:
|
||||
dest = dest_input
|
||||
else:
|
||||
dest = os.path.abspath(dest_input)
|
||||
if not dest.startswith(tmpdir + os.sep):
|
||||
print(
|
||||
f"vale-wrap.sh: refusing to lint '{src}': its scratch copy would "
|
||||
f"land outside {tmpdir}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(2)
|
||||
os.makedirs(os.path.dirname(dest), exist_ok=True)
|
||||
|
||||
# surrogateescape keeps a non-UTF-8 file (reachable via a directory argument)
|
||||
# a byte-for-byte round trip instead of aborting the whole run on a decode error.
|
||||
with open(src, encoding='utf-8', errors='surrogateescape') as fh:
|
||||
@@ -435,6 +459,9 @@ if header_m:
|
||||
|
||||
with open(dest, 'w', encoding='utf-8', errors='surrogateescape') as fh:
|
||||
fh.write(content)
|
||||
|
||||
if tmpdir is not None:
|
||||
print(dest)
|
||||
PYTHON
|
||||
}
|
||||
|
||||
@@ -449,23 +476,23 @@ mkdir -p "$mirror"
|
||||
argv_paths=()
|
||||
for arg in ${path_args[@]+"${path_args[@]}"}; do
|
||||
if [[ "$arg" == /* ]]; then
|
||||
dest="$tmpdir$arg"
|
||||
raw_dest="$tmpdir$arg"
|
||||
else
|
||||
dest="$mirror/$arg"
|
||||
raw_dest="$mirror/$arg"
|
||||
fi
|
||||
dest="$(abspath "$dest")"
|
||||
# A path argument with enough leading `..` to climb past the mirror root would
|
||||
# write outside the scratch dir. The real filesystem clamps such a path at
|
||||
# `/`; the mirror can't, so refuse rather than scribble outside the sandbox.
|
||||
case "$dest" in
|
||||
"$tmpdir"/*) ;;
|
||||
*)
|
||||
echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
mkdir -p "$(dirname "$dest")"
|
||||
if [[ -d "$arg" ]]; then
|
||||
dest="$(abspath "$raw_dest")"
|
||||
# A path argument with enough leading `..` to climb past the mirror root would
|
||||
# write outside the scratch dir. The real filesystem clamps such a path at
|
||||
# `/`; the mirror can't, so refuse rather than scribble outside the sandbox.
|
||||
case "$dest" in
|
||||
"$tmpdir"/*) ;;
|
||||
*)
|
||||
echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
mkdir -p "$(dirname "$dest")"
|
||||
# A directory is mirrored whole — vale applies its own format filtering to
|
||||
# the tree, so any file dropped here would be silently unlinted — and then
|
||||
# every markdown file in the copy is flattened in place. `.git` is pruned:
|
||||
@@ -484,7 +511,9 @@ for arg in ${path_args[@]+"${path_args[@]}"}; do
|
||||
flatten "$md" "$md"
|
||||
done < <(find "$dest" -type f -name '*.md' -print0)
|
||||
else
|
||||
flatten "$arg" "$dest"
|
||||
# `abspath` + `flatten` folded into one python3 process — see the comment
|
||||
# atop `flatten` above.
|
||||
dest="$(flatten "$arg" "$raw_dest" "$tmpdir")"
|
||||
fi
|
||||
if [[ "$arg" == /* ]]; then
|
||||
argv_paths+=("$dest")
|
||||
|
||||
Reference in New Issue
Block a user