feat: add tcsh support#17
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds tcsh support and a tcsh test harness; refactors generator.sh to use per-shell inline templates for escaping and control flow, replaces awk color parsing with while/read loops, consolidates inline generation for Rainbow/Reset blocks, and appends nested-color test outputs to existing shell test scripts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Gen as generator.sh
participant FS as Filesystem
participant Shells as Target Shells (sh/bash/zsh/ksh/fish/tcsh)
participant TestRunner as test-scripts/*
Dev->>Gen: run generator.sh
Gen->>FS: read color table files
FS-->>Gen: return color lines
Gen->>Gen: parse with while/read -> colorCode, labels
Gen->>Gen: select per-shell templates (escape/if/startSym/endSym/para)
loop generate per target shell
Gen->>FS: write ColorEcho.<shell> using inline templates
Gen->>FS: append Rainbow/Reset conditionals using shell template
end
Note right of Shells: tcsh uses alias-based handlers and single-line conditionals
Dev->>TestRunner: run test-scripts/<shell>
TestRunner->>FS: (for tcsh) generate runtime tcsh script mapping aliases
TestRunner->>Shells: pipe or invoke shell to execute tests
Shells->>FS: source ColorEcho.<shell> and run echo tests (including nested-color output)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @moormaster, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||||||
PR Code Suggestions ✨Latest suggestions up to 58d242d
Previous suggestions✅ Suggestions up to commit 6820bcd
|
||||||||||||||||||||||||||||||
![gemini-code-assist[bot]](https://ghub.bbstale.com/avatars/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for tcsh by refactoring the generator.sh script. The refactoring significantly improves the generator's performance by replacing inefficient grep and awk calls in a loop with a single cat | while read construct, which is a great improvement. However, the changes introduce a critical bug in the echo.Reset function for all shells due to an incorrect tr command, and a sed syntax error in echo.Reset specifically for tcsh. Additionally, the generated code for echo.Rainbow has become less readable for shells that support multi-line functions. I've provided comments with suggestions to fix these issues.
| cat << LOLCAT >> "${tempDist}" | ||
| ${fnName}${startSym} | ||
| ${ifCond} | ||
| echo "\$${para}" | lolcat | ||
| else | ||
| echo "\$${para}" | ||
| ${endIf} | ||
| ${endSym} | ||
| ${fnName}${startSym}${if}${ifCond}${then}echo "${para}" | lolcat${else}echo "${para}"${endIf}${endSym} | ||
| LOLCAT |
There was a problem hiding this comment.
Condensing the if-then-else-fi logic into a single line for echo.Rainbow reduces readability in the generated scripts for shells like bash, zsh, fish, and ksh. While this is necessary for tcsh aliases, it would be better to handle tcsh as a special case and retain the more readable multi-line format for other shells.
You could achieve this with something like:
if [ "${shell}" = "tcsh" ]; then
cat << LOLCAT >> "${tempDist}"
${fnName}${startSym}${if}${ifCond}${then}echo "${para}" | lolcat${else}echo "${para}"${endIf}${endSym}
LOLCAT
else
# The old multi-line cat << LOLCAT block
cat << LOLCAT >> "${tempDist}"
${fnName}${startSym}
${if}${ifCond}${then}
echo "${para}" | lolcat
${else}
echo "${para}"
${endIf}
${endSym}
LOLCAT
fiA similar change should be applied to echo.Reset generation for consistency and readability.
![coderabbitai[bot]](https://ghub.bbstale.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
generator.sh (1)
1-244: Address ShellCheck suggestions and apply shfmt to generator.sh
- Distribution files are up-to-date; no unstaged changes under dist/.
- ShellCheck reports only info-level warnings (SC1091, SC1003, SC2016); adjust quoting/escaping per the suggestions.
- Install/configure shfmt and reformat generator.sh with
shfmt -sr -i 2 -cito enforce consistency.
🧹 Nitpick comments (3)
test-scripts/tcsh (2)
5-5: Add existence check for ColorEcho.tcsh.The script should verify that the ColorEcho.tcsh file exists before attempting to source it, to provide a clearer error message if the file is missing.
Apply this diff:
dir="$(cd "$(dirname "$0")" && pwd)" script="${dir}/../dist/ColorEcho.tcsh" + +if [ ! -f "${script}" ]; then + echo "Error: ${script} not found. Please run generator.sh first." >&2 + exit 1 +fi
8-13: Consider adding test output for better feedback.The test silently executes each alias without providing feedback on success or failure. Consider adding output to confirm which aliases were tested.
{ echo "source \"${script}\"" + echo "echo 'Testing ColorEcho aliases for tcsh...'" awk '/^alias +echo/ {print $2}' "${script}" | while IFS= read -r x; do - echo "${x} \"${x}\"" + echo "echo 'Testing: ${x}'" + echo "${x} \"${x}\"" done + echo "echo 'All tests completed.'" } | tcshgenerator.sh (1)
165-165: Consider removing useless use of cat.The current pattern
cat "${table}" | while read -rcan be simplified towhile read -r ... done < "${table}"to avoid the unnecessary cat process.Apply this diff:
- cat "${table}" | while read -r color colorCode; do + while read -r color colorCode; do # light or not for light in "" "Light"; do if [ "${light}" = "" ]; then code=3 else code=9 fi # style like bold, italic, underline, blinking, strikethrough for style in "" "Bold" "I" "UL" "BL" "ST"; do case "${style}" in "Bold") styleCode='1;' ;; "I") styleCode='3;' ;; "UL") styleCode='4;' ;; "BL") styleCode='5;' ;; "ST") styleCode='9;' ;; "" | *) styleCode= ;; esac for style2 in "" "Bold" "I" "UL" "BL" "ST"; do if [ "${style}" != "${style2}" ]; then case "${style2}" in "Bold") finalStyleCode="${styleCode}1;" ;; "I") finalStyleCode="${styleCode}3;" ;; "UL") finalStyleCode="${styleCode}4;" ;; "BL") finalStyleCode="${styleCode}5;" ;; "ST") finalStyleCode="${styleCode}9;" ;; "" | *) finalStyleCode="${styleCode}" ;; esac else style2= finalStyleCode="${styleCode}" fi echoFunction="$(printf "%secho%s%s%s%s%s" "${fn}" "${dot}" "${light}" "${style}" "${style2}" "${color}")" if ! grep -q "${echoFunction}" "${tempDist}"; then { echo "" printf "%s%s" "${echoFunction}" "${brackets}" # write the code down echo "${startSym}${echo} "'"'"${escape}033[${finalStyleCode}${code}${colorCode}m${para}${escape}033[m"'"'"${endSym}" } >> "${tempDist}" fi done done done - done + done < "${table}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
dist/ColorEcho.bashis excluded by!**/dist/**dist/ColorEcho.fishis excluded by!**/dist/**dist/ColorEcho.kshis excluded by!**/dist/**dist/ColorEcho.shis excluded by!**/dist/**dist/ColorEcho.tcshis excluded by!**/dist/**dist/ColorEcho.zshis excluded by!**/dist/**
📒 Files selected for processing (2)
generator.sh(5 hunks)test-scripts/tcsh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
@(generator.sh|color table.txt)
📄 CodeRabbit inference engine (AGENTS.md)
Regenerate the distribution scripts with ./generator.sh whenever generator.sh or color table.txt is modified; commit the resulting dist/ColorEcho.* updates together
Files:
generator.sh
**/*.{sh,bash,ksh,zsh,fish,md,markdown}
📄 CodeRabbit inference engine (AGENTS.md)
Honor 2-space indentation (and .editorconfig defaults) for shell and Markdown files
Files:
generator.sh
**/*.{sh,bash,ksh,zsh,fish}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{sh,bash,ksh,zsh,fish}: Format shell scripts with shfmt using flags -sr -i 2 -ci to align with CI
Run ShellCheck on modified shell scripts
Files:
generator.sh
generator.sh
📄 CodeRabbit inference engine (AGENTS.md)
Keep helper names and files consistent with the generator (echo.<Style>); add new helpers only via generator.sh
Files:
generator.sh
🧠 Learnings (2)
📚 Learning: 2025-09-29T15:56:53.647Z
Learnt from: CR
PR: PeterDaveHello/ColorEchoForShell#0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T15:56:53.647Z
Learning: Applies to generator.sh : Keep helper names and files consistent with the generator (echo.<Style><Color>); add new helpers only via generator.sh
Applied to files:
generator.sh
📚 Learning: 2025-09-29T15:56:53.647Z
Learnt from: CR
PR: PeterDaveHello/ColorEchoForShell#0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T15:56:53.647Z
Learning: Applies to @(generator.sh|color table.txt) : Regenerate the distribution scripts with ./generator.sh whenever generator.sh or color table.txt is modified; commit the resulting dist/ColorEcho.* updates together
Applied to files:
generator.sh
🔇 Additional comments (8)
test-scripts/tcsh (2)
1-6: LGTM!The shebang, error handling, and path resolution are correct and follow best practices.
1-13: ShellCheck OK; shfmt not available
- ShellCheck reports no issues for test-scripts/tcsh.
shfmtcommand not found: install shfmt and runshfmt -sr -i 2 -ci -d test-scripts/tcshto confirm formatting per guidelines.generator.sh (6)
32-32: LGTM!The addition of tcsh to the shell list is correct.
37-119: LGTM!The per-shell configuration variables (escape, if, then, else, endIf, startSym, endSym, para) are correctly defined for each shell. The values are consistent with each shell's syntax requirements.
220-222: LGTM!The tcsh-specific conditional check using
whichand tcsh-style redirection (>& /dev/null) is appropriate for tcsh compatibility.
85-102: Manual verification required for tcsh configuration.The sandbox lacks tcsh; please confirm in a native tcsh shell that:
- ANSI escape sequences work with plain
echo- Parameter substitution via
\!*in aliases passes all arguments&&and||operator precedence yields the intended conditional flow
213-230: tcsh Rainbow alias logic is correct The pipe|binds tighter than&&and||in tcsh, so the alias implements the intended fallback. (web.pa.msu.edu)
203-203: Inline template generates correct escape sequences and quoting for bash, ksh, tcsh, and fish; no changes needed.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
dist/ColorEcho.bashis excluded by!**/dist/**dist/ColorEcho.fishis excluded by!**/dist/**dist/ColorEcho.kshis excluded by!**/dist/**dist/ColorEcho.shis excluded by!**/dist/**dist/ColorEcho.tcshis excluded by!**/dist/**dist/ColorEcho.zshis excluded by!**/dist/**
📒 Files selected for processing (1)
generator.sh(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
@(generator.sh|color table.txt)
📄 CodeRabbit inference engine (AGENTS.md)
Regenerate the distribution scripts with ./generator.sh whenever generator.sh or color table.txt is modified; commit the resulting dist/ColorEcho.* updates together
Files:
generator.sh
**/*.{sh,bash,ksh,zsh,fish,md,markdown}
📄 CodeRabbit inference engine (AGENTS.md)
Honor 2-space indentation (and .editorconfig defaults) for shell and Markdown files
Files:
generator.sh
**/*.{sh,bash,ksh,zsh,fish}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{sh,bash,ksh,zsh,fish}: Format shell scripts with shfmt using flags -sr -i 2 -ci to align with CI
Run ShellCheck on modified shell scripts
Files:
generator.sh
generator.sh
📄 CodeRabbit inference engine (AGENTS.md)
Keep helper names and files consistent with the generator (echo.<Style>); add new helpers only via generator.sh
Files:
generator.sh
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: PeterDaveHello/ColorEchoForShell#0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T15:56:53.647Z
Learning: Use the scripts under test-scripts/ to validate the corresponding shell variant; rerun all variants after generator or color changes
| ifCond='which lolcat >& /dev/null' | ||
| # tcsh uses aliases instead of functions -> replace ', [ and ] with quoted chars | ||
| trCntrl='\\x27\[:cntrl:\]\\x27' | ||
| ;; |
There was a problem hiding this comment.
tcsh Reset alias deletes apostrophes
trCntrl='\\x27\[:cntrl:\]\\x27' expands to tr -d \x27[:cntrl:]\x27, so the generated tcsh alias removes literal ' (0x27) along with control chars. echo.Reset must not drop regular characters from user text. Please emit the tcsh set as just [:cntrl:], which works inside the single-quoted alias without eating apostrophes.
- trCntrl='\\x27\[:cntrl:\]\\x27'
+ trCntrl='[:cntrl:]'🤖 Prompt for AI Agents
In generator.sh around lines 223-226, trCntrl is set to '\\x27\[:cntrl:\]\\x27'
which expands to tr -d \x27[:cntrl:]\x27 and causes tcsh aliases to remove
literal apostrophes from user text; change the emitted tcsh set to just
'[:cntrl:]' so the alias is defined without surrounding \x27 quotes (i.e.,
assign trCntrl to the plain character class and ensure the alias generation uses
that value inside the single-quoted tcsh alias so apostrophes are preserved).
There was a problem hiding this comment.
Even worse - tcsh takes \x27 literally and removes \, x, 2 and 7 ... fixed that.
There was a problem hiding this comment.
@moormaster, thanks for catching that additional detail and fixing it! You're absolutely right—tcsh would interpret \x27 as four literal characters to remove (\, x, 2, 7) rather than as a hex escape sequence for the apostrophe character. The corrected version with just [:cntrl:] avoids this issue entirely and ensures only control characters are removed from the output, preserving all regular characters including apostrophes.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
moormaster
force-pushed
the
feat/add-tcsh
branch
from
24089bd to
58d242d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds tcsh shell support to ColorEcho, implementing color echo functions using aliases instead of function declarations since tcsh doesn't support functions. The changes include refactoring the generator to use condensed one-line declarations across all shells and improving cross-shell character escaping compatibility.
Key changes:
- Add tcsh shell support with alias-based color functions using
\!*parameter syntax - Refactor generator to produce one-line function/alias declarations for all shells
- Update character escaping patterns for better cross-shell compatibility
Reviewed Changes
Copilot reviewed 2 out of 8 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| generator.sh | Add tcsh support and refactor to generate condensed one-line declarations |
| dist/ColorEcho.tcsh | New tcsh implementation with 1270+ color aliases |
| test-scripts/tcsh | Test script to validate tcsh alias functionality |
| dist/ColorEcho.{bash,ksh,sh,zsh,fish} | Updated to use condensed one-line format |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,13 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
The shebang should use tcsh instead of bash since this is a tcsh test script. Using bash shebang conflicts with the tcsh commands used later in the script.
| #!/usr/bin/env bash | |
| #!/usr/bin/env tcsh |
There was a problem hiding this comment.
This is a bash script generating tcsh commands and piping them into tcsh.
| startSym=' { | ||
| ' | ||
| endSym=' | ||
| }' |
User description
See Issue #2
PR Type
Enhancement
Description
Add tcsh shell support with alias-based color functions
Refactor generator to support one-line function declarations
Update existing shell outputs to use condensed format
Fix character escaping for cross-shell compatibility
Diagram Walkthrough
File Walkthrough
generator.sh
Add tcsh support and refactor generatorgenerator.sh
ColorEcho.tcsh
New tcsh color echo implementationdist/ColorEcho.tcsh
aliasinstead offunctiondeclarations\!*ColorEcho.bash
Refactor to one-line declarationsdist/ColorEcho.bash
trcommandColorEcho.ksh
Refactor to one-line declarationsdist/ColorEcho.ksh
trcommandColorEcho.sh
Refactor to one-line declarationsdist/ColorEcho.sh
trcommandColorEcho.zsh
Refactor to one-line declarationsdist/ColorEcho.zsh
trcommandColorEcho.fish
Refactor to one-line declarationsdist/ColorEcho.fish
trcommandtcsh
Add tcsh testing scripttest-scripts/tcsh
Summary by CodeRabbit