Aegisub
Aegisub copied to clipboard
[LUA] fix - Lua Scripts that change the cwd also affect Aegisub
Lua Scripts that change the cwd can also affect Aegisub, here I save the cwd before the registering of the Lua scripts and then restore it afterwards, for more see #166
A Problem that I encountered is, that these scriptsrun asynchronus, so the exact time when a possible cwd change happens, is unknown, so a better way to solve this would to pass the saved start cwd to each component, or make it available in main, so that it can be guaranteed, that it is the one, aegisub got started in
Another problem with this approach is that automation scripts may change the working directory whenever they execute code, which is also when their macros are executed, making this approach not exactly stable.
A better approach would be not to use relative paths to the workding dir in the code at all or as little as possible, resolving relative paths explicitly to a known directory instead. Except for loading files from the command line, I don't think any of the current features rely on the working directory.
The second approach saves the start cwd and makes it accessible to all those, who want it, that happens only once, so you are right, but scripts can modify the cwd only when they're run: 1: at startup, 2: at manual execution of the script in the automation tab. So the second approach is 100% secure and we "need" the cwd for relative path resolvinf, it's only needed for opening a .ass, but it's more conveninet to open it from the terminal like : aegisub ./test.ass rather than aegisub $(realpath ./test.ass) or aegisub /really/long/absolute/path/test.ass
I admit I haven't checked the PR contents (and notably the second commit) and only read the comments here in the PR. This approach is indeed what I described and what should be used. Ideally it would also use startCwd in all the places where relative paths are currently supported to explictly tie them together, but I know practically nothing about the codebase to make any more substantial comments currently.
As far As I found and knew, thats the only place it's used explicitly, there may be other calls then current_path() that use the cwd, thats also out of my knowledge about that
The CI fails are related to #162