Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add custom shell flag option for command execution #3726

Closed
wants to merge 3 commits into from
Closed

feat: Add custom shell flag option for command execution #3726

wants to merge 3 commits into from

Conversation

LangLangBart
Copy link
Contributor

@LangLangBart LangLangBart commented Apr 14, 2024

proposal

Ability to alter the flag that is passed to the $SHELL command.


For example, if the -c flag could be altered to -e, one could use jsc (JavaScriptCore) to execute JavaScript code without adding jsc -e to every line.

# List available globals in 'jsc' and preview their properties
# 'jsc' only comes by default on macOS
SHELL=/System/Library/Frameworks/JavaScriptCore.framework/Versions/A/Helpers/jsc \
  FZF_SHELL_FLAG="-e" \
  fzf \
  --bind 'start:reload:print(Object.getOwnPropertyNames(globalThis).join("\n"))' \
  --bind 'enter:become:print((![]+[])[+[]]+([][[]]+[])[+[]]+([][[]]+[])[+!![]])' \
  --preview 'const f = globalThis[{}]; if (f) { print(`{} Static Properties\n${Object.getOwnPropertyNames(f).sort().join("\n")}`); if (f.prototype) print(`\n{} Prototype Properties\n${Object.getOwnPropertyNames(f.prototype).sort().join("\n")}`)}'

Another example using bun and --print.

# [bun] https://github.com/oven-sh/bun
# brew install oven-sh/bun/bun
# [gh] https://github.com/cli/cli
# Show starred repos and preview the date of the first commit in a Korean time format
user_name=gh api graphql -f query='{viewer{login}}' --jq '..|.login? // empty'
gh api users/${user_name}/starred \
  --jq '.[] | select(.visibility == "public") | .full_name' |
  SHELL="$(which bun)" FZF_SHELL_FLAG="--print" \
    fzf --prompt "Starred [${user_name}]" \
	--preview '((repo)=>fetch(`https://api.github.com/repos/${repo}/commits?per_page=1`).then((i)=>fetch(i.headers.get("link")?.match(/http[^>]+/g)?.at(-1)).then((t)=>t.json().then((S)=>`First commit: ${repo}\n${new Intl.DateTimeFormat("ko-Kr",{timeStyle:"full",dateStyle:"full"}).format(new Date(S[0].commit.author.date))}`))))({})'

Pro:

  • Enables the execution of commands without requiring the flag and command to be prepended on every
    line, which also diminishes the need for one round of quoting.

Con:

  • It may lead to confusion and inconsistencies if not used carefully and documented clearly.

Issues

  • Writing tests
  • Windows (not tested at all)
  • Better documention

@LangLangBart LangLangBart marked this pull request as draft April 14, 2024 09:23
@junegunn
Copy link
Owner

junegunn commented Apr 14, 2024

Thanks, but I'm not sure about this. Correct me if I'm wrong, but we can achieve the same thing by setting $SHELL to a simple wrapper script that handles the flags, which I think is not bad.

fzf2() (
  export SHELL=/tmp/fzfsh
  if ! [[ -x $SHELL ]]; then
    echo '#!/bin/sh' > "$SHELL"
    echo 'shift; echo "$@"' >> "$SHELL"
    chmod +x "$SHELL"
  fi
  command fzf "$@"
)

fzf2 --preview 'run this command: {}'

Having this option may save a few lines of code in some cases, but I'm afraid it will be a niche feature that few will use.

@junegunn
Copy link
Owner

The Windows situation is a bit messy now, and adding support for this might make it more complicated.

func ExecCommandWith(shell string, command string, setpgid bool) *exec.Cmd {
var cmd *exec.Cmd
if strings.Contains(shell, "cmd") {
cmd = exec.Command(shell)
cmd.SysProcAttr = &syscall.SysProcAttr{
HideWindow: false,
CmdLine: fmt.Sprintf(` /v:on/s/c "%s"`, command),
CreationFlags: 0,
}
return cmd
}
if strings.Contains(shell, "pwsh") || strings.Contains(shell, "powershell") {
cmd = exec.Command(shell, "-NoProfile", "-Command", command)
} else {
cmd = exec.Command(shell, "-c", command)
}
cmd.SysProcAttr = &syscall.SysProcAttr{
HideWindow: false,
CreationFlags: 0,
}
return cmd
}

@LangLangBart
Copy link
Contributor Author

simple wrapper script that handles the flags

Your wrapper script is a simple and effective solution. No need for a new feature that few would utilize.

@junegunn
Copy link
Owner

that few would utilize

Technically, this was just my guess, so I can't be 100% sure. But if I recall correctly, there hasn't been a feature request for this in the past years. But if you still like the idea, we can keep this PR open for a while and see what others think.

IMO, if we're really going to add this, it should be easier to use than 1. setting two environment variables, 2. or creating a wrapper script. Something like fzf --with-shell "$SHELL -c" maybe? Hmm, then we would have another problem of splitting the arguments without the help from a shell. (e.g. fzf --with-shell 'custom-shell --title "hello fzf" -c')

@LangLangBart
Copy link
Contributor Author

I can't reopen the PR as the repo was deleted, but I will file an issue report.

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