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

[question/feature request] Allow slashes (/) in flag keys #315

Closed
wildefires opened this issue Jan 28, 2020 · 5 comments
Closed

[question/feature request] Allow slashes (/) in flag keys #315

wildefires opened this issue Jan 28, 2020 · 5 comments

Comments

@wildefires
Copy link

Expected Behavior

Flags with the / character should be able to be made

Current Behavior

util.IsSafeKey() rejects flags with a slash due to the keyRegex defined as ^[\w\d-]+$

Possible Solution

Change the regex to allow slashes

Context

Trying to namespace flags with slashes.

Question

Is there a technical reason for the lack of slashes here? If not, it should be a fairly innocuous change that I'd gladly put up a PR for

@zhouzhuojie
Copy link
Collaborator

I see, are you going to use / as a higher order of separator than -? I think it makes sense to change the regex to fit this.

@wildefires
Copy link
Author

Yeah, the plan is to encourage our users to namespace things in intelligent ways. Something like

service-name/flag1
service-name/flag2

or even

team-name/service-name/flag1

@wildefires
Copy link
Author

wildefires commented Jan 28, 2020

I'm working on the necessary change to this now - the simplest route is to change the regex to ^[\w\d-/]+$. My hesitation with this approach is that it allows for leading and trailing slashes as well (e.g. /myflagname/). Is that an issue, do you think?

@zhouzhuojie
Copy link
Collaborator

I think that's ok, people may like some file-path style if they choose to.

@wildefires
Copy link
Author

Closed by #316

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

No branches or pull requests

2 participants