Repository navigation
Conversation
760ee73 to
2820f15
Compare
There was a problem hiding this comment.
I am lightly concerned about the performance loss (many new function calls), but if the consistency is worthwhile that is likely fine. We only manage on the order of hundreds of files in a single conversion run.
The names of the new functions are fine actually - they fit the other pathname_* functions. If we wanted to be explicit that these are file test we can consider adding a _filetest_ piece. So pathname_filetest_r. Or we can go even shorter and do filetest_r? But if we were to add a new term, I'd use the one from the perl documentation.
|
Also, could you please rebase the PR to the master branch? Windows CI should be passing again. |
dginev
left a comment
dginev
left a comment
There was a problem hiding this comment.
Circling back here (while Bruce is still "on holiday"), I still think we need a better naming strategy for the testing subroutines.
pathname_f just doesn't evoke any useful meaning to me. filetest_f does - so maybe that is a reasonable change? Or a name on those lines?
I mean that for the subs that do the perl -dash tests, specifically.
|
I don't have deep knowledge of the code, but I did notice this PR. Could some of this be done by bringing in It won't solve the encoding problem, but it may help with the API design. |
In the meanwhile I changed it to |
|
In the meanwhile, I have implemented some portable wrappers around open, test, etc in LaTeXML can actually depend directly on Win32::LongPath (which is now inclueded by default in Strawberry Perl) and simplify the code further, without that klugde with the Win32 module, which works, but only if the filesystem supports short paths. |
This is an attempt at refactoring all I/O calls that deal with file/directory names through wrapper functions in Util::Pathname. This means all the
-Xtests,open,opendir,stat, andunlink. There might be others I did not catch.My goal is to guarantee that normal LaTeXML code is never exposed to raw file names, which are not Unicode strings and must be decoded and encoded before use. The enconding and decoding will be done in the wrapper functions, although not yet – I first want to know if this refactor is ok.
(The encoding and decoding itself is simple, see #2545 for Unix using Encode::Locale. For Windows, I concluded that the only way to go is Win32::LongPath.)