Skip to content

Four of configure.sh's Five SC2016 Disables Suppress Nothing, and the Skill Citing It Describes the Wrong Property #1156

Description

@ptr727

Found by the local diff review on the PR for #1075/#1120, which stripped the directives and re-ran the linter rather than reading the file. Filed rather than patched, because it needs a re-measure and then a decision, and because my first attempt at fixing it in that PR made the carried claim worse.

The measurement

Strip all five # shellcheck disable=SC2016 directives from repo-config/configure.sh and run shellcheck. Exactly one SC2016 is emitted, at the norm='def w(f): . as $in ...' single-quoted assignment near line 303.

The other four directives, at lines 124, 311, 313 and 345, suppress nothing:

  • shellcheck exempts single-quoted arguments passed to jq itself, so a --arg call needs no disable.
  • Lines 311 and 313 sit above double-quoted jq strings using \$t, where SC2016 (which fires only on single quotes) never applies at all.

Why it matters

A directive that suppresses nothing is indistinguishable from one that does, so a real SC2016 appearing later in that region is silently swallowed with no reader able to tell which of the five was load-bearing. The rule the file is cited as illustrating exists precisely to keep a considered exception legible.

The carried claim is wrong too

.agents/skills/shell-codestyle/SKILL.md "Rules" says:

repo-config/configure.sh is the worked example, carrying five SC2016 disables where a single-quoted jq program must stay unexpanded, each with its reason on the same line.

Two defects, and they pull in opposite directions:

  1. "five" and "each with its reason on the same line" were both false, because line 313 carried a bare directive with no reason, which is what the rule's own first half forbids.
  2. "a single-quoted jq program" is the right property for the one directive that fires and wrong for the two double-quoted ones.

In the PR I first "fixed" this by dropping "single-quoted" and adding the missing reason comment to line 313. That was backwards: single-quoting is exactly what triggers SC2016, so the reworded claim described lines with no single quotes at all, and the added comment justified an inert directive. Both changes are reverted, and the text is back to its original wording, which is closer to true.

What settling it needs

Remove the four directives that suppress nothing, confirm shellcheck stays clean, then make the skill's claim match the count and the property that remain. Doing it in the other order re-vendors a wrong rule to ~22 carriers.

Not This Issue

  • #1155 covers other gate-reach gaps found in the same review arc.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    scriptA defect in hub tooling

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions