Repository navigation
ENH - (Windows) Support aas_expandpathbyvars for Windows #289
Description
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
Activity
tiborauer commentedon Nov 8, 2021
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 commentedon 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 commentedon 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 commentedon 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.
Do not do aas_expandpathbyvars on windows