Pass the filepath to parse()

[andrewdeandrade]

I would like to write a parser that resolves any values that are relative paths (starts with ./ or ../ and fs.existsSync returns true) to absolute paths. The parser would need to know the location of the file in order to be able to resolve a potential path and confirm the file exists.

I would submit a PR for this today, but I'm still trying to figunavigatere my company's open source contribution policy.

[dominictarr]

@malandrew we have just recently officially removed this feature. If you want to make changes to rc you need to make a really solid case about why this is a worthwhile change, and how you intend to use it.
This sets a very high bar for contributions, but this is what keeps rc an exceptionally simple and useful module. I cannot promise I will merge your pull request, but I will gladly listen to your arguments for why it may be diseriable.

[blakeembrey]

@dominictarr I believe I have the same requirement - I want to be able to load CA files for proxies when specified in the rc file, but without knowing where the file originally come from it's difficult. I'm not really interested in parsing, but just resolving certain values (as paths). What about exposing utils and passing file to the parse function as the second argument?

Edit: Or even a normalize function that just returns the input by default, but can be overridden?

[dominictarr]

@blakeembrey so, reading between your lines, you have other files related to your config that you want to load - is the location of these files specified in the config files or are they just in a specific location relative to the config files?

[blakeembrey]

Yes, relative to the rc file and specified in the rc file.

[dominictarr]

hmm... well, loading those files is not something that should happen in a parse function.
@blakeembrey are you using multiple config files? If I was to do that I'd just put them in the ~/.{appname}/ directory and then have my app assume they live there. I don't think having a overriding prototype chain style config would make that much sense in this case....

or maybe... rc could interpret relative file names into absolute file names when it loads that rc source...

[blakeembrey]

@dominictarr No, loading shouldn't happen there - I was only suggesting resolving to the absolute path could occur there. As for where things are stored, I guess I could enforce it - it's just not right now.

[dominictarr]

oh okay, yeah resolving to the absolute path could work, but it could also be problematic because possiblely there are already people using configurations with things that look like relative files but aren't meant to be interpreted like that... I think it would have to be a major version

[blakeembrey]

I don't know about magical parsing of things that look like paths. If you don't want to include the filename as part of the parse step, what about providing a validate step that can take the result and return a new one? That way validation can occur of each file (E.g. check for strings/numbers/etc) and it remains backward compatible.

[blakeembrey]

Oh, I'm open to doing a PR for each to show the difference - I believe supporting the parser is less of a big change, but I don't mind writing a demonstration.

[dominictarr]

Yeah, I'm a bit torn here, definately having absolute paths in a config file is a pain, but changing everything that looks like an relative path to be an absolute one. That said, I havn't felt a strong need for filepaths in my own configs, and when I do need something like that I've just made a config directory.

Also, rc wants to be unconfigurable.

What I'd really like to see is another person who needs this feature, and what their use case looks like.

[legodude17]

My PR (#85) will make it so this is not needed for extension specific parsing.

[blakeembrey]

That looks really complex and fragile. A much simpler solution is to:

1. Expose parse with the public API
2. Pass the filename to parse (instead of parse(fileConfig) make it parse(fileConfig, file))
3. As the module consumer, wrap the exposed parse function (E.g. function custom (content, file) { var res = parse(content, file); /* Do something withres*/ return res; })

Edit: To clarify, I never submitted a PR because it was stated that someone else needed to be interested.

[legodude17]

Why do you think it is really fragile? Can you point out specific places? Also, can you please move discussion of the PR to the PR itself?

[blakeembrey]

@legodude17 It just introduces more complexity that isn't really required in the core module. For instance, if you received the filename in the parse function all this can be implemented in user land without any changes needed (perhaps just making extensions a new option in the future). Also, won't you now be doing x*n file lookups (where n is the extension)? I'm not the maintainer, I just suggested the minimal change that I would make to support this specific feature that you commented in - your feature is actually completely unrelated to this issue though so my initial reaction was out of context.

[legodude17]

I commented here because mine is a different solution than this one. If you don't like, your dislike is noted, I can't please everyone.

[blakeembrey]

It's not related at all. Did you actually read this issue? It's about resolving filenames inside the configuration.