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.

Sign up for GitHub

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

Jump to bottom

Argument names only allow lowercase ascii #33

Closed
3 of 4 tasks
jamesread opened this issue Jan 6, 2022 · 2 comments
Closed
3 of 4 tasks

Argument names only allow lowercase ascii #33

jamesread opened this issue Jan 6, 2022 · 2 comments
Labels
progress: next-release type: bug Something isn't working

Comments

@jamesread
Copy link
Collaborator

jamesread commented Jan 6, 2022 •
edited

Describe the bug
Argument names only allow lowercase ascii, so arguments like docker_image, or dockerImage will not work (but dockerimage) will.

Thanks to MyQe on Discord for finding the issue.

Expected behaviour
docker_image and dockerImage should both be supported as valid argument names.

Workaround
This issue will be added to the next release to fix. For now, a workaround is changing arguments that are formatted like dockerImage and docker_image to just dockerimage.

Plan to fix

  • Extend list of valid characters allowed in an argument name. internal/executor/executor.go
  • Check that command does not exec unless all args are replaced.
  • Create tests for various formats of variable name.
  • Fatal error on config reload if a valid argument name is found.
@jamesread jamesread added type: bug Something isn't working progress: next-release labels Jan 6, 2022
jamesread added a commit that referenced this issue Apr 5, 2022
@jamesread
bugfix: Issue #33 - argument names now allow a-zA-Z0-9_
17c1804
@jamesread
Copy link
Collaborator Author

Decided against the fatal error on config reload if an invalid argument name is found. Crashing for that case seems unnecessary.

jamesread added a commit to OliveTin/docs.olivetin.app that referenced this issue Apr 5, 2022
@jamesread
Detail on valid arg names, OliveTin/OliveTin#33
32c36c6
@jamesread jamesread added progress: code-done Code is done and removed progress: code-done Code is done labels Apr 5, 2022
@jamesread
Copy link
Collaborator Author

Thank was released in https://github.com/OliveTin/OliveTin/releases/tag/2022-04-07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
progress: next-release type: bug Something isn't working
Milestone
No milestone
Development

No branches or pull requests
1 participant
@jamesread