Skip to content

fix(rewrite): skip cat rewrite when incompatible flags present#847

Open
MauroDeryckere wants to merge 1 commit intortk-ai:developfrom
MauroDeryckere:fix/cat-flag-rewrite
Open

fix(rewrite): skip cat rewrite when incompatible flags present#847
MauroDeryckere wants to merge 1 commit intortk-ai:developfrom
MauroDeryckere:fix/cat-flag-rewrite

Conversation

@MauroDeryckere
Copy link

Summary

  • Skip catrtk read rewrite when cat is invoked with flags that have different semantics than rtk read (e.g. -A, -v, -e, -t, -s, --show-all)
  • Allow rewrite for cat -n since it maps correctly to rtk read -n (both mean line numbers)
  • Plain cat file continues to rewrite to rtk read file as before

Problem

rtk rewrite blindly forwards all cat flags to rtk read. This causes incorrect behavior because the flags have different semantics:

Flag cat meaning rtk read meaning
-n Line numbers Line numbers (compatible)
-v Show non-printing chars Verbosity level (incompatible)
-A Show all (non-printing + endings) Not supported (fails)
-e Show non-printing + $ at EOL Not supported
-t Show non-printing + tabs as ^I Not supported
-s Squeeze blank lines Not supported

For example, cat -A file.cpp was rewritten to rtk read -A file.cpp which fails with rtk: Failed to resolve 'read' via PATH.

Fix

Early check in rewrite_segment(): if the command starts with cat and the first argument starts with - (but is not -n), return None to skip the rewrite. This follows the same pattern used for head -N and tail special cases.

Test plan

  • New test test_rewrite_cat_with_incompatible_flags_skipped — verifies -A, -v, -e, -t, -s, --show-all all return None
  • New test test_rewrite_cat_with_compatible_flags — verifies cat -n file.txt rewrites to rtk read -n file.txt
  • Existing test_rewrite_cat_file still passes — plain cat filertk read file
  • Full test suite: 1119 passed, 3 ignored

cat flags like -v, -A, -e, -t, -s have different semantics than
rtk read flags (-v means verbose, -n means line numbers). Blindly
forwarding flags caused incorrect behavior (e.g. cat -A file →
rtk read -A file which fails).

Only skip rewrite for incompatible flags. cat -n (line numbers) maps
correctly to rtk read -n, so it is still rewritten for token savings.
Plain `cat file` continues to rewrite to `rtk read file` as before.
@pszymkowiak pszymkowiak added bug Something isn't working effort-small Quelques heures, 1 fichier labels Mar 26, 2026
@pszymkowiak
Copy link
Collaborator

[w] wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type bug-fix
🟢 Risk low

Summary

Fixes the cat-to-rtk-read rewrite to skip commands with incompatible flags (e.g., -A, -v, -e, -t, -s, --show-all) that have different semantics in rtk read, while still allowing the compatible -n flag. Previously, cat with unsupported flags was blindly rewritten, causing rtk read failures.

Review Checklist

  • Tests present
  • Breaking change
  • Docs updated

Analyzed automatically by wshm · This is an automated analysis, not a human review.

@aeppling
Copy link
Contributor

Hey

We are cleaning up the codebase and improving the project structure for better onboarding. As part of this effort, PR #826 reorganizes src/ from a flat layout into subfolders.

No logic changes — only file moves and import path updates.

What you need to do

Rebase your branch on develop when receiving this comment:

git fetch origin && git rebase origin/develop

Git detects renames automatically. If you get import conflicts, update the paths:

use crate::git;        // now: use crate::cmds::git::git;
use crate::tracking;   // now: use crate::core::tracking;
use crate::config;     // now: use crate::core::config;
use crate::init;       // now: use crate::hooks::init;
use crate::gain;       // now: use crate::analytics::gain;

Need help rebasing? Tag @aeppling

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

Labels

bug Something isn't working effort-small Quelques heures, 1 fichier

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants