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

Help with sh: #16

Closed
lucas-mior opened this issue Apr 15, 2020 · 2 comments · Fixed by #17
Closed

Help with sh: #16

lucas-mior opened this issue Apr 15, 2020 · 2 comments · Fixed by #17

Comments

@lucas-mior
Copy link
Contributor

I can't make the following work:

audio/* sh: mediainfo "%s" | awk -F' {2,19}:' '{printf("%s : %s\n",$1,$2)}'

The code works in zsh. I've tried changing awk's quoting to double quotes and escaping quotes and dollar signs, but any of that did.
What I am missing?

@doronbehar
Copy link
Owner

Thank you for reporting this. Indeed the new sh: capability was far from perfect and it didn't have the ability to handle usages such as this. As for a fix to your experience, which I'd like to deliver as fast as possible, I'm realizing now it may require config format changes, not only when sh: is used.

You see, in the old way Pistol operates, %s is replaced using fmt.Sprintf with the file name. This is problematic because of 2 reasons:

  1. If sh: is used, file names with spaces & other characters aren't quoted / escaped properly before the shell runs the command.
  2. Users may want to use %s in the shell and have it not replaced by Pistol before passed down to the shell, as in your awk script.

Specifically for mediainfo, it seems that it doesn't spit an error if you give it a not-found filename argument which makes (1) much harder to discover when trying to debug this.

To fix (2), I realize now that I should have used a different placeholder for the file name replacement right from the start, when I started working on Pistol... I mean: instead of using fmt.Sprintf, use perhaps %pistol-filename% (which would be reserved for pistol and no other program such as awk will use it, naturally) and use strings.ReplaceAll instead.

You can try out this new feature at #17 .

@lucas-mior
Copy link
Contributor Author

Thanks!
Now it works just fine with the following:
audio/* sh: mediainfo %pistol-filename% | awk -F' {2,19}:' '{printf("%s : %s\n",$1,$2)}'

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 a pull request may close this issue.

2 participants