A refactor todo list

[jlaurens]

Refactoring is necessary to make stronger code.

This week I have spent a very big amount of time in refactoring the code.
It ends up in either dozens of small PRs or just one big PR that is absolutely intractable because diff cannot relate it to the original code.
None is reasonable.

The midway is to use a small amount of PR's, each one being dedicated to one kind of refactoring.
@josephwright It would be efficient to have a refactor branch where PR's can be merged after a review but before a full latex2e test.
Then once in a while, this branch can be merged to master if appropriate.

Things to do

0. Replace build_require by require (and explicit the "build.lua is first" ambiguity)
1. cosmetic changes: rename variables, spaces after commas...
2. remove these nasty _ variables (new "util" module)
3. move some global methods to the local module scope
4. separate os related, file system related, path related, glob related stuff (prepare enhancements)
5. Reorganize "sanity checks"
6. Explicit code dependencies: more require statements, complete the local print = print safeguard list,
7. Mark all global variables in the _G namespace (to verify that no more variable is incidentally global and to know exactly what are the global variables)
8. local code refactor: use table.concat
9. Document the code (for development mainly),
10. manage quoted strings
11. Return the correct number of values (abspath and gsub return 2 values)
12. ...

Forthcoming changes:

Readonly tables

Readonly tables add runtime checks to prevent errors.

Arguments

• turn "arguments" into a module
• simplify code with select and no long vs short split
• --no-<flag> support
• explicit the target → option map
• hide implementation details: declare_option function.

Globs

• replace glob_to_pattern by a glob_matcher: local accept = glob_matcher(<glob>); if accept(<file name>) then ... end`
• implement unix like globs
• global option to exclude dot files

Walk

For path management

• objects to implement paths properly
• sanitize what comes from the build.lua (and config)
• ensure that path are cross platform (no UTF8, no forbidden windows characters, no ";")
• uniform path formatting
• implement path equality: solve the "./." == "." problem (see filelist)
• instead of using builddir: use a proxy to return builddir when defined or a default value (computed properties)

Global variables

In code, use computed properties instead of global variables.

Lua unit testing

There are 3 absolutely different levels

• for l3buid itself
• for l3build clients (including package authors)
• inside luatex documents

[wspr]

I agree the refactoring is important for long term code viability. Why can't a PR be made with a large number of atomic commits that can be inspected? A single diff view from before and after is likely to be impossible to understand, as you say, but I didn't think PRs were too problematic as long as all of the commits within the PR are well structured and titled/described.

Out of curiosity, what is wrong with build_require() ? It was implemented in contrast to require() to avoid errors or security issues arising from matching incompatible or malicious files in other directories. In other words, enforce that all required components were located in the same directory as l3build.lua itself.

[jlaurens]

I may have missed something, is it possible to review a single commit inside a PR online?
Anyway, reviewing a PR with many commits may take too much time before merge.

Nothing wrong with build_require implementation, this is more like a shallow rename. The purpose is to turn it into a global function with exactly the same restrictions to allow the classical schemes

local lfs = require("lfs")
local foo = require("l3b.foo")

from any file of the package, including build.lua and friends.
Moreover this will clarify the file dependency graph and prepares for unit testing.

[josephwright]

simplify code with select and no long vs short split

There is a difference: short arguments are -X<par> whereas long ones are --X-can-include-Y [=] <par>

[jlaurens]

Of course, no UI change at all, only some implementation changes.
Otherwise it is not really refactoring.

[josephwright]

refactor branch

No issue there although in the end that will still have to have a big PR to master.

[jlaurens]

Or we can merge refactor into master.

[josephwright]

implement unix like globs

As has been noted before, standard globs are Windows ones: Windows is the most popular desktop OS in the world. Moreover, there's never been a need to go beyond what * and ? offer: as it is, those were questionable vs requiring a fixed list of files. I really don't see a need to go further than the current support.

[jlaurens]

It seems that Microsoft does not completely agree with you.

https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_wildcards?view=powershell-7.1

[josephwright]

ensure that path are cross platform (no UTF8, no forbidden windows characters, no ";")

Indeed but that does mean " is banned, as are a few other things [although I don't see the need to filter out LPT1 or whatever ;)].

[josephwright]

or l3build clients (including package authors)

I'm still confused here: l3build is (deliberately) self-contained, and if someone wants to use ideas from it, they would make a copy and modify as required.

[jlaurens]

I just wanted to point out the different situations.

1. l3build is being developed and thus cannot be used to completely test itself
2. clients use a working l3build (or clone) to test external files (actual usage)
3. authors want to test some lua code embedded in a tex file. No simple solution yet.