Skip to content

Use the new snip run and check subcommands#13

Open
JoaoCostaIFG wants to merge 3 commits into
VincentHardouin:mainfrom
JoaoCostaIFG:main
Open

Use the new snip run and check subcommands#13
JoaoCostaIFG wants to merge 3 commits into
VincentHardouin:mainfrom
JoaoCostaIFG:main

Conversation

@JoaoCostaIFG
Copy link
Copy Markdown

2 new subcommands were added to snip:

  • run - safer way to specify the command to be run
  • check - check if a command should be run with snip. For example, shell built-ins like cd and export would be rejected.

This PR:

  • Replaces hardcoded unproxyable list with a runtime call to snip check
  • Deduplicate findFirstPipe call - the AST was traversed twice
  • Replace console.warn with client.log - per opencode recommendations (console.warn in plugin code mangles the UI)

… list

Replace the hardcoded UNPROXYABLE_COMMANDS set with runtime snip check
calls to determine whether a command should be wrapped. Commands that
pass the check are prefixed with 'snip run --' instead of 'snip'.
This delegates shell builtin detection to snip itself and makes the
plugin extensible via snip's filter configuration.
@VincentHardouin
Copy link
Copy Markdown
Owner

Thanks for your contribution ! I’ve seen your contribution on snip repo, they haven’t been released yet. I’ll take a closer look at your PR when I have a little more time. At a quick glance, I think we’ll need to check the installed version of snip to prompt users to update it.

@JoaoCostaIFG
Copy link
Copy Markdown
Author

That's a good point, didn't think about that. I'll try to add a check for the snip version to preserve the previous behavior when using older version of snip (without the run/check sub-commands)

@VincentHardouin
Copy link
Copy Markdown
Owner

That's a good point, didn't think about that. I'll try to add a check for the snip version to preserve the previous behavior when using older version of snip (without the run/check sub-commands)

I don't think it's important to maintain backward compatibility. I'm more in favor of making sure that people using opencode-snip@latest are aware that they need to update snip.

@harshcurious
Copy link
Copy Markdown

Hey folks, any updates on this?

@lenucksi
Copy link
Copy Markdown

This PR has been superseded by PR #14 (#14), which builds on your foundational work adding snip check/`snip run` support and client.log replacement.

PR #14 additionally includes two critical fixes that were discovered while testing:

1. Multi-word snip check arguments: snip check -- "git log -10" returns no filter (one string arg). The command is now split into individual words: snip check -- git log -10 correctly returns filter: git-log.

2. All pipe segments are checked: Previously only the first command before | was evaluated. Now every pipe segment is independently resolved via snip check, with proper quote awareness and compound operator handling within each segment.

Thank you @JoaoCostaIFG for your contribution — the new PR includes full credit for your work in the Acknowledgements section.

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.

4 participants