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

Files are relative to cwd, not include files #39

Open
ndmitchell opened this issue Sep 8, 2023 · 5 comments
Open

Files are relative to cwd, not include files #39

ndmitchell opened this issue Sep 8, 2023 · 5 comments

Comments

@ndmitchell
Copy link

If I have args/file1.txt with:

@file2.txt

And args/file2.txt with:

--foo

I would expect running a command line with @args/file1.txt would see --foo as the files inside args/file1.txt should be interpreted relative to args (the directory the file is in). Currently all the files are interpreted relative to the cwd.

@ndmitchell
Copy link
Author

If there is agreement that this is the right way to go, I'd be happy to submit a PR.

@epage
Copy link
Contributor

epage commented Sep 8, 2023

Please include reproduction cases. By the fact that you are doing recursive loading, I'm going to make the assumption you are using parse_fromfile. In this case, we are specifically modeling the semantics off of another library and that libraries behavior is the expected behavior. If I'm reading the preference implementation correctly, it is always against CWD.

However, the design allows users to provide their own policy for how to treat the response files so you can make it relative to the file being processed.

@ndmitchell
Copy link
Author

Thanks for the info, I am indeed using parse_fromfile. Makes sense to default to working the same way, given that is how the other library works.

If I did want to make it load relative to the current path, I don't see any way to do that. The expand_args_from takes the contents and the prefix and contents, but doesn't get made aware of the path, so it doesn't have any way to resolve that paths as relative to the current file. This could be fixed by passing Path to the parser, but I can't think of any other way as most filenames aren't exposed to the users of this library.

@epage
Copy link
Contributor

epage commented Sep 8, 2023

Right, instead of replacing parse_fromfile, you'd like instead replace expand_args_from, tracking the base path.

@ndmitchell
Copy link
Author

OK, fair enough. It's a bit of a shame that expand_args_from is a fairly heavyweight function, so I have to copy/paste a lot of it.

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