Skip to content

fix(refacto-p2): more standardize#910

Merged
aeppling merged 7 commits intorefacto-folders-and-documentationfrom
refacto-files-and-constants
Mar 30, 2026
Merged

fix(refacto-p2): more standardize#910
aeppling merged 7 commits intorefacto-folders-and-documentationfrom
refacto-files-and-constants

Conversation

@aeppling
Copy link
Copy Markdown
Contributor

Summary

  • constant extract of repetitive patterns
  • more covering for run_filtered wrapper and sub wrappers for specifics cmds environments
  • automod for cmds environments (auto discover)
  • merge pattern & rules for routing

Test plan

  • all test pass
  • all edge cases kept (exclusions not covered by global wrappers)

@aeppling aeppling force-pushed the refacto-folders-and-documentation branch from c7db13d to 48ec28d Compare March 29, 2026 12:22
@aeppling aeppling force-pushed the refacto-files-and-constants branch from c09127d to 8a277e3 Compare March 29, 2026 12:48
Copy link
Copy Markdown
Collaborator

@FlorianBruniaux FlorianBruniaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three things to fix before merging.

1. Silent JSON fallback in container.rs (run_kubectl_json)
When serde_json::from_str fails, the closure returns stdout.to_string() with no warning. Users get raw kubectl JSON (potentially thousands of tokens) with no indication the filter failed. Add eprintln!("[rtk] kubectl: JSON parse failed: {}", e) in the Err branch.

2. Savings inflation in runner.rs (filter_stdout_only path)
When filter_stdout_only is true, text_to_filter = &stdout but timer.track() is called with &raw (stdout + stderr combined) as the input. Savings % gets inflated by stderr length. On noisy commands like docker build, stderr can be substantial. The fix: track &stdout as raw input when filter_stdout_only is true. This is pre-existing but it is now centralized in one place, so the right time to fix it is here.

3. automod contributor contract change
Every .rs file dropped into src/cmds/system/ is now automatically a public module. The previous pub mod foo; declarations were a forcing function. A developer who adds a WIP file directly into the directory will silently expose it. Low risk, but worth a comment or a note in the README explaining that automod is in use.

The run_aws_filtered deduplication (180 lines to 60) and the run_passthrough centralization are both good. Depends on #904, merge that first.

aeppling and others added 7 commits March 30, 2026 17:57
- ENV_PREFIX now handles FOO="bar baz", FOO='val', escaped quotes
- Move 6 inline lazy_static regex to module level
- Merge rewrite_head_numeric + rewrite_tail_lines → rewrite_line_range
- Consolidate head/tail call sites in rewrite_segment
- Add rewrite pipeline documentation to TECHNICAL.md
- Rewrite discover README as concept/flow documentation
- Add lexer-based tokenizer for shell command parsing

Co-Authored-By: Andrew Hundt <ATHundt@gmail.com>
- container.rs: warn on kubectl JSON parse failure instead of silent fallback
- runner.rs: fix savings inflation when filter_stdout_only by tracking stdout only
- README.md: document automod behavior in module registration step
@aeppling aeppling force-pushed the refacto-files-and-constants branch from 8a277e3 to 0a8b8fd Compare March 30, 2026 16:14
@aeppling
Copy link
Copy Markdown
Contributor Author

Rebased with refacto-folders-and-documentation and review issues resolved @FlorianBruniaux

@aeppling aeppling merged commit 92c671a into refacto-folders-and-documentation Mar 30, 2026
1 check passed
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.

2 participants