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

freeze --execute doesn't work with pipes/redirects #67

Open
jacksonmcafee opened this issue Apr 3, 2024 · 3 comments
Open

freeze --execute doesn't work with pipes/redirects #67

jacksonmcafee opened this issue Apr 3, 2024 · 3 comments

Comments

@jacksonmcafee
Copy link

jacksonmcafee commented Apr 3, 2024

Feature / Bug

I really like the --execute functionality of freeze but I am somewhat annoyed with the fact that I cannot execute commands with pipes or redirects. To provide a simple example, I created a file named file.txt with the contents:

bar1
foo1
bar2
foo2

cat file.txt | grep 'foo' will return:

foo1
foo2

freeze --execute "cat file.txt | grep 'foo'" will return:
freeze

It appears to stops parsing at the pipe and only executes cat file.txt. I checked shellwords.Parse(), which is called by pty.go, executeCommand(), and the behavior might stem from there.

I have no experience in Go so I can't be sure, but someone more knowledgeable than me might be able to verify. When I added test cases to check this in the parsing library, it appears to parse up until a pipe or redirect as expected. Apologies if my analysis is wrong, but in my cursory look at the code, that's what stood out to me.

Describe the solution you'd like

It would be nice if --execute would take any command and execute it to completion before creating the output image. Ideally, in the example above, that command should function identically and return the following image:
freeze

Describe alternatives you've considered

To get the above image, I used cat file.txt | grep 'foo' | freeze --language bash. This isn't too complex of an alternative, but has two immediate downsides:

  • It requires that the user pass a language.
  • It seems to go against the initial instinct to use --execute in the first place.

This also highlights that syntax highlighting will not work on --executed inputs. If I wanted to do something like freeze --execute "cat main.cpp", it would fail to properly highlight the syntax. I don't really think this is that problematic, but it is worthy of note. The --language flag will not fix this and will output the same image.

Additional context

I am using a non-standard freeze config but I can't imagine that's affecting anything in this case. I just like less padding & rounded corners on my images :)

@jacksonmcafee jacksonmcafee changed the title freeze --execute doesn't work with pipes/redirects freeze --execute doesn't work with pipes/redirects Apr 3, 2024
@prithvijj
Copy link

Just wanted to add some findings!

I added in some print statements near

freeze/pty.go

Lines 30 to 38 in 80803eb

args, err := shellwords.Parse(config.Execute)
if err != nil {
return "", err
}
ctx, cancel := context.WithTimeout(context.Background(), config.ExecuteTimeout)
defer cancel()
cmd := exec.CommandContext(ctx, args[0], args[1:]...)
pty, err := config.runInPty(cmd)

image

Assuming there is a file:

# hi.txt

bar1
foo1
bar2
foo2

When you run

go run ./... --execute "cat hi.txt | grep foo"

It only recognizes upto []string{"cat", "hi.txt"} and ignores the pipe/redirect and just executes the command

/usr/bin/cat hi.txt

If you try to add escape characters to the pipe such that it's

go run ./... --execute "cat hi.txt \| grep foo"

It recognizes the entire command []string{"cat", "hi.txt", "|", "grep", "foo"}, However when it tries to execute the command

/usr/bin/cat hi.txt | grep foo

It throws an error, since I think that exec standard lib doesn't really process it (Referencing this link https://medium.com/@didi12468/use-exec-command-to-execute-a-command-with-a-pipe-b05e5087bf8a)

The fix mentioned was to use bash -c 'cat hi.txt | grep foo' (basing on the above link) such that when we run

go run ./... --execute "bash -c 'cat hi.txt | grep foo'"

It recognizes the command []string{"bash", "-c", "cat hi.txt | grep foo"}, And successfully executes the given command

image

@GianniBYoung
Copy link

Behavior surrounding --execute seems to have lots of issues in its current state. Barring the current lack of ability to print the command itself (see #81)) alongside the output I ran into similar issues with parsing a command.

freeze -x 'echo -e "git ls-remote --get-url origin\n" && git ls-remote --get-url origin'

yields the strange result of -e git ls-remote --get-url originn indicating various struggles.

The expected result is

git ls-remote --get-url origin

[email protected]:project/project.git

@prithvijj
Copy link

prithvijj commented Apr 26, 2024

Behavior surrounding --execute seems to have lots of issues in its current state. Barring the current lack of ability to print the command itself (see #81)) alongside the output I ran into similar issues with parsing a command.

freeze -x 'echo -e "git ls-remote --get-url origin\n" && git ls-remote --get-url origin'

yields the strange result of -e git ls-remote --get-url originn indicating various struggles.

The expected result is

git ls-remote --get-url origin

[email protected]:project/project.git

Seems like if i did

freeze ./... -x "bash -c 'echo -e \"git ls-remote --get-url origin\n\" && git ls-remote --get-url origin'"

gets you that picture for now!

image
But yeah based on more testing, like it really fails on just characters like ; && | etc,

For an input that is go run ./... -x 'echo -e "git ls-remote --get-url origin\n" && git ls-remote --get-url origin'
It only recognizes []string{"echo", "-e", "git ls-remote --get-url originn"},
And executes /usr/bin/echo -e git ls-remote --get-url originn

pbj@pop-os:~/Github/freeze2$ go run ./... -x 'echo -e "git ls-remote --get-url origin\n" && git ls-remote --get-url origin'
[]string{"echo", "-e", "git ls-remote --get-url originn"}
/usr/bin/echo -e git ls-remote --get-url originn

but the bash -c "<command>" seems to work, which would be similar to this function https://github.com/caarlos0/go-shellwords/blob/05672174625ca58db2f54d29354ac8e0e0ad2eea/util_posix.go#L12-L17, being used in go-shellwords library

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

No branches or pull requests

3 participants