Skip to content

Separate execution traces from error output exactly, and filter by PS4 only when it is literal - #133

Merged
mernst merged 3 commits into
plume-lib:masterfrom
mernst:cronic-ps4
Oct 9, 2026
Merged

mernst merged 3 commits into
plume-lib:masterfrom
mernst:cronic-ps4

Conversation

@mernst

@mernst mernst commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

cronic removes execution-trace lines from the wrapped command's error output, using a pattern built from PS4. The pattern was built by inserting PS4 into a regular expression without escaping, so:

  • a PS4 with regular-expression metacharacters, such as [trace] , did not match its own trace lines, which were then reported as errors;
  • a PS4 with an expansion, such as +${LINENO}: , was matched literally, though its expansion differs from line to line. Any looser pattern for such a PS4 could also match, and hide, a real error such as +++ b/file.

This pull request:

  • under bash 4.1 or later, sends the execution trace to a separate file via BASH_XTRACEFD, so trace lines are separated from the error output exactly, whatever PS4 is. The trace appears in the report under "TRACE OUTPUT". The file descriptor is allocated by bash, so it does not collide with descriptors that the wrapped command uses;
  • for other shells, which write the trace to stderr, still removes lines that start with PS4, but:
    • escapes PS4's metacharacters before building the pattern;
    • repeats a multibyte first character of PS4 as a whole, whatever the locale;
    • filters trace lines only when PS4 contains no $, backquote, backslash, or newline. Otherwise, the trace lines are reported along with the rest of the error output, so that no real error is hidden;
  • documents the treatment of execution traces in cronic --help;
  • adds tests for these cases. The tests of PS4 in stderr are skipped when run as root, because bash then ignores PS4 from the environment.

This is split out of #129.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 54 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: plume-lib/plume-scripts/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: d95af882-d7e9-4d73-846e-7b360a7d05ba
📥 Commits

Reviewing files that changed from the base of the PR and between 78d062f and 63cea8c.

📒 Files selected for processing (2)
  • cronic
  • tests/cronic-test/cronic-test.sh
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

mernst added a commit to mernst/plume-scripts that referenced this pull request Oct 9, 2026
mernst added 2 commits October 8, 2026 20:47
# Conflicts:
#	cronic
#	tests/cronic-test/cronic-test.sh
Bash 4.1 or later writes its trace to a separate file, which the report shows under "TRACE OUTPUT".  For other shells, trace lines are still removed from stderr by PS4; a multibyte first character of PS4 is repeated as a whole, and a PS4 containing a newline is not used for filtering.
@mernst mernst changed the title Filter trace lines only when PS4 is literal, and escape its metacharacters Separate execution traces from error output exactly, and filter by PS4 only when it is literal Oct 9, 2026
@mernst
mernst merged commit 1fe58e5 into plume-lib:master Oct 9, 2026
7 checks passed
@mernst
mernst deleted the cronic-ps4 branch October 11, 2026 20:25
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