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
Make scripts in config.yml relative to config.yml #4860
Conversation
ee31f1b
to
2e7d234
Compare
2e7d234
to
18a0791
Compare
4478012
to
0afacc5
Compare
f0383ea
to
7352811
Compare
54dd690
to
9f8d4a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks! 🍰
I've left some very nitpicky comments below, looks great overall! 😃
No super strong opinion here, I just felt that script_path(...,
relative_to=x) is very readable. Feel free to take it as a regular arg if
you prefer that. :)
…On Sat, Dec 2, 2023, 18:47 Lucas Ficheux ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In mitmproxy/optmanager.py
<#4860 (comment)>:
> +def compute_path(script_path: str, config_path: str) -> str:
+ """
+ Make relative paths found in config files relative to said config file,
+ instead of relative to where the command is ran.
+ """
+ if os.path.isabs(script_path):
+ return script_path
+ if script_path != os.path.expanduser(script_path):
+ return script_path
+ return os.path.abspath(os.path.join(os.path.dirname(config_path), script_path))
I'll get on it right away.
Just a quick question, why make relative_to a kw_only argument here ?
—
Reply to this email directly, view it on GitHub
<#4860 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHY2PXCTRHPBWYCDN4UT23YHMIMJAVCNFSM5F5UA2PKU5DIOJSWCZC7NNSXTPCQOVWGYUTFOF2WK43UKJSXM2LFO45TCNZWGA4DMMJVGQ3A>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
f2176da
to
73213f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Relative $HOME is cursed, great catch. 😅
ed58f33
to
c2b0a60
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick turnaround! 😃
Make the paths to scripts found in config.yml relative to the directory where config.yml is instead of where the command is ran.
825e59f
to
37a9b3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, fantastic work! 😃 🚀 🚀 🚀
Description
Make the paths to scripts found in config.yml relative to
the directory where config.yml is instead of where the command
is ran.
Checklist