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

ENH - (Windows) Support aas_expandpathbyvars for Windows #289

Open
AljenU opened this issue Nov 8, 2021 · 4 comments
Open

ENH - (Windows) Support aas_expandpathbyvars for Windows #289

AljenU opened this issue Nov 8, 2021 · 4 comments

Comments

@AljenU
Copy link
Collaborator

AljenU commented Nov 8, 2021

Currently aas_expandpathbyvars does nothing on Windows. Thus paths in the xml cannot use environment variables like %APPDATA%.
Should path expansion also be supported on Windows? Does need to take into account also #288

The subfunction expandind would change from

ind = sort([strfind(x,'$'),strfind(x,'`')]);

to e.g.

if ispc()
    % Even more unsafe than on non-pc, since % is likely to be found in formatting expressions
    ind = sort(strfind(x,'%'));
else
    ind = sort([strfind(x,'$'),strfind(x,'`')]);
end
@tiborauer
Copy link
Member

It is not a core goal but is worth noting as a TODO for later.
However, it seems you, @AljenU, have already found a clever solution. :)

@AljenU
Copy link
Collaborator Author

AljenU commented Nov 9, 2021

It basically works indeed. However, because the % is also used a lot in printf formatting statements, there is even more chance of accidental replacements (#288). Thus i would be more comfortable if there were some improvements for #288

@tiborauer
Copy link
Member

tiborauer commented Nov 9, 2021

How about using $ for Windows, as well? I know it is not a direct mapping and might be a little confusing for the users, but some explicit documetation would help for sure.

@AljenU
Copy link
Collaborator Author

AljenU commented Nov 9, 2021

On windows, there needs to be a % at the start and at the end of the environment variable that is to be replaced. So the analogy with the unix case would get even more confusing.
And to deviate from standard conventions should have a really good reason, since it is easy to accidentally fall back to default behaviour, which would then lead to errors.
I did just add some idea for fixing #288, in which case it would also be ok to enable this one with the easy change from the starting comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants