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

pass extra arguments (#51) #55

Merged
merged 1 commit into from
Mar 13, 2021
Merged

pass extra arguments (#51) #55

merged 1 commit into from
Mar 13, 2021

Conversation

lucas-mior
Copy link
Contributor

  • change cmdline options
  • pass extras to NewPreviewer
  • parse %pistol-extra[0-9]+% from config file
  • note on README about extra args

adresses #51.
thanks for reviewing it.

@doronbehar
Copy link
Owner

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
Copy link
Owner

@lucas-mior
Copy link
Contributor Author

lucas-mior commented Mar 11, 2021

Done.

@lucas-mior
Copy link
Contributor Author

lucas-mior commented Mar 11, 2021

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 :)

Fixes #51. Teach Pistol to accept extra arguments to be passed to the
commands defined in the config. If the matched line in the config
doesn't use `%pistol-extra[0-9]%` the argument is discarded.

Update documentation and add tests.
@doronbehar
Copy link
Owner

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.

@doronbehar doronbehar merged commit e27055f into doronbehar:master Mar 13, 2021
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