search

doronbehar / pistol

General purpose file previewer designed for Ranger, Lf to make scope.sh redundant
MIT License
321 stars 7 forks source link

pass extra arguments (#51) #55

Closed lucas-mior closed 3 years ago

lucas-mior commented 3 years ago

adresses #51. thanks for reviewing it.

doronbehar commented 3 years ago

Thanks a lot for the patch @lucas-mior ! I added tests and edited the documentation to explain the feature better as I had issues myself understanding it at first :). Could you please read the current version of the README in your branch? I'd like to add there an example usage for image previews that works for you currently.

Also, I noticed you use 2 different email addresses for committing changes - in general it'd be nice for you to add them both to your github account so that it'll be easy to trace you in GitHub blame.

doronbehar commented 3 years ago

The feature is explained here: https://github.com/lucas-mior/pistol/blob/master/README.adoc#passing-arbitrary-extra-arguments-to-commands

lucas-mior commented 3 years ago

Done.

lucas-mior commented 3 years ago

In the doc you added, you mention that one could pass up until 9 extra arguments, which is not the case. Currently pistol will pass any argument matched with %pistol-extra([0-9]+)% (if such argument is passed to pistol). I just don't know if you did that because it's more desirable to do the former (limit to 9)? Feel free to change that :)

doronbehar commented 3 years ago

In the doc you added, you mention that one could pass up until 9 extra arguments, which is not the case. Currently pistol will pass any argument matched with %pistol-extra([0-9]+)% (if such argument is passed to pistol). I just don't know if you did that because it's more desirable to do the former (limit to 9)? Feel free to change that :)

Did not intend to change / suggest changing that and I'm fine with having no limit. I misinterpreted what the code does.

Done.

Thanks again :) If in the future, you'd have a more targeted script that does what pv does for images, it'd be nice to update the README to mention it.