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

Allow placeholder in tags #23

Open
bomgar opened this issue Apr 26, 2017 · 8 comments
Open

Allow placeholder in tags #23

bomgar opened this issue Apr 26, 2017 · 8 comments

Comments

@bomgar
Copy link
Member

bomgar commented Apr 26, 2017

Would help a lot to set override-path for go projects

@bomgar bomgar changed the title Allow placeolder in tags Allow placeholder in tags Apr 26, 2017
@mriehl
Copy link
Member

mriehl commented May 5, 2017

@bomgar do you still need this?

@bomgar
Copy link
Member Author

bomgar commented May 5, 2017

yes. because i have to specify a go tag per github group. with a placehoder that won't be nescessary

@mriehl
Copy link
Member

mriehl commented May 6, 2017 via email

@mriehl
Copy link
Member

mriehl commented Aug 16, 2017

@bomgar now that you have integrated lua in fblog I think we could consider doing that here too. Spawn a lua func with all sorts of stuff as locals and use the value produced by the func as result.

Maybe this could look like

"override_path": "lua: strlower(github_repo)"

Performance might become an actual concern because we have to evaluate most of the config greedily on load (to ensure sanity for instance)

@LordMZTE
Copy link
Contributor

I don't think using an embedded interpreter is necessary here.
What could also be a much easier and more convenient option is to actually just use a custom binary, and pipe the supplied string into stdin with relevant data being set in environment variables.
This would allow for usage like this:

# using bash
override_path = "/bin/sh: echo ${repo_name,,}"

# using lua
override_path = '/usr/bin/lua: print(strlower(os.getenv("repo_name")))'

These would both return the name of the repo to lower case.
I think this would allow for more flexibility, and if we're gonna add the prefix, then we might as well use a path to a binary to give users more freedom and allow any scripting language.

Maybe we could also use a shebang like syntax for this, and then write the script to a file in /tmp:

# using bash
override_path = '''
#!/bin/sh
echo ${repo_name,,}
'''

# using lua
override_path = '''
#!/usr/bin/lua
print(strlower(os.getenv("repo_name")))
'''

@bomgar
Copy link
Member Author

bomgar commented Jun 13, 2021

I'm not convinced executing arbitrary code is a good thing here. Maybe a template language like handlebars would be sufficient. But I'm not sure yet.

@LordMZTE
Copy link
Contributor

Well, a template language would still not allow complicated operations, such as lower casing a string, or converting from camel case to snake_case for example (unless we manually add those functions, but we can't cover everything). Besides that, the proposed lua functionality would also allow executing arbitrary code unless it's sandboxed. I also don't really see a security concern here, as this is a user's personal config which should be trusted.

@bomgar
Copy link
Member Author

bomgar commented Jun 14, 2021

Embedded lua would be fine I guess. Calling external processes adds a lot of complexity to everything and is harder to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants