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

genders file to include other files #23

Open
stuartm-dugeo opened this issue Jun 15, 2018 · 8 comments
Open

genders file to include other files #23

stuartm-dugeo opened this issue Jun 15, 2018 · 8 comments

Comments

@stuartm-dugeo
Copy link
Contributor

Would be great to have the ability for nested genders files...

eg. /etc/genders to be

include /etc/genders_c1
include /etc/genders_c2

name1 attr1,attr2=a

This would allow us to have 1 genders file manage multiple clusters.

@BenCasses
Copy link
Contributor

I hear that this concept was discussed early in the creation of genders. It's a good idea that would at least cut down on mistakes. Pushback at the time, reasonably, was: "feature-creep". I'm going to add this to the TODO list and pick it up when I have cycles.

@stuartm-dugeo
Copy link
Contributor Author

Thanks.

@chu11
Copy link
Member

chu11 commented Jul 3, 2018

One comment/thing to ponder, if an old version of genders parsed

include /etc/genders_c1

I think it would see a node named "include" and an attribute named "/etc/genders_c1".

Perhaps it should be:

#include /etc/genders_c1

So atleast the include directive is not read b/c its a comment in earlier versions.

Or are we just introducing an alternate unexpected behavior so it doesn't matter either way?

@garlick
Copy link
Member

garlick commented Jul 3, 2018

Least surprise might be to have the new include directive cause a parse error in old genders versions, if that's even possible - looks like we are pretty liberal about what constitutes a valid line of input.

@BenCasses
Copy link
Contributor

I agree that it should fail visibly when using an older version of genders. Whoever used the include probably did it on purpose and should have a strong indication of badness rather than some genders silently disappearing if include is interpreted as a comment. Seems like a 3-word line breaks the parser. Maybe something like

include gender /etc/genders_c1

@garlick
Copy link
Member

garlick commented Jul 10, 2018

I'm not sure there is a way to make this pretty, but would it be useful to introduce a special directive prefix character so that future versions of the library can add directives beyond "include", and unknown directives can be detected and called out as such?

For example, maybe use

% include /etc/genders_c1

then in new parser

%include /etc/genders_c1
Line 1: % directive must be followed by white space

% foo bar
Line 1: % foo is an unknown directive

as opposed to

include /etc/genders_c1
(not detected)

include genders /etc/gendres_c1
Line 1: white space in attribute list

@baallan
Copy link

baallan commented Jul 19, 2018

I've had good results (and more easily debugged results) just concatenating various input pieces with application logic into a single input file before parsing genders.

@baallan
Copy link

baallan commented Jul 19, 2018

If you go with "include genders /etc/gendres_c1" syntax, for completeness consider
include genders
include pipe [other args]
where the second reads the output of system() calling program with the args.

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

5 participants