Relative paths with Filename template for config files in non-standard locations

[arroyoj]

First of all, thank you for a great library.

I ran into an issue using relative paths in yaml files with the Filename template. In my program, I'm allowing the user to override the config.yaml found in the configuration directories with a per-run yaml file specified on the command line. The per-run yaml file is loaded as an additional YamlSource that overlays the user and default config files. However, if a relative file path is used in this additional config file and then validated with the Filename template, the returned path is relative to the output of config_dir() and not the directory name of the yaml file itself. Here is an example, using ConfigSource to mock loading a yaml file:

import confuse
config = confuse.Configuration('test')
file_source = confuse.ConfigSource({'foo': 'foo/bar'},
                                   filename='/baz/config.yaml')
config.set(file_source)
valid_foo_path = config['foo'].get(confuse.Filename())
print(valid_foo_path)

I had expected the value of valid_foo_path to be /baz/foo/bar, but it will be something like /home/<username>/.config/test/foo/bar, based on whatever the output of config_dir() is. This comes from these lines in the value() method of Filename: https://github.com/beetbox/confuse/blob/046ecb143875920e9162483b08f429d24f24d5fe/confuse/templates.py#L546-L548

I realize this behavior is documented in the Filename docstring, which says that relative paths will be relative to the configuration directory if they come from a file, but this does not seem intuitive for config files that lie outside of the configuration directory.

Could the Filename template be changed to make relative paths relative to the directory containing the config file, rather than the value of config_dir()? For example, replacing the above with something like this?

    elif self.in_app_dir:
        # From defaults: relative to the app's directory.
        path = os.path.join(view.root().config_dir(), path)
    elif source.filename:
        path = os.path.join(os.path.dirname(source.filename), path)

For user config.yaml files already in the configuration directory, I don't think this would have any impact. In addition, it would allow the default config_default.yaml file to easily refer to other default files included with the package. However, the above change would alter the current behavior for config files that lie outside of the configuration directory. If backwards compatibility needs to be retained, maybe we could make the new behavior opt-in by adding an in_source_dir=False parameter to Filename.__init__() that would force the above behavior? This would mirror the in_app_dir parameter that forces resolving the path relative to the app configuration directory.

I'd be happy to put together a PR to implement either of these.

[sampsyo]

Hi! This seems like a great idea, one way or another---it definitely seems desirable to let filenames be relative to wherever the config file may live.

I guess I want to recommend that we do the backwards-compatible version, at least at first. I can definitely imagine that there are beets users, for example, that rely on extra config files that contain relative paths that work the same way as their "main" config file, i.e., paths are relative to the same base directory.

It occurs to me that the opt-in to the new behavior could occur at two points: the template or the source itself. It might be a little cleaner (from the client side) to do the latter, i.e., to add an option to YamlSource to change the behavior. I'm not immediately sure how complicated that would be, but maybe it's worth a shot?

[arroyoj]

Thanks for the feedback and suggestion about implementing the behavior at the level of the source itself. In #126, I implemented both the source-level opt-in for the entire file and the template-level opt-in for an individual configuration value. I think there are some advantages to each one, but I'm happy to simplify it down to one if you think that would be better.

[arroyoj]

Closing now that #126 has been merged.