use diff -u?

we should make this a hidden module, no?

also, this should be configurable, people should be able to disable the backup if they're confident enough (but I guess having it enabled by default makes sense)

Making it a hidden module makes sense just for TCL modules. I had lua modules in mind, but since there are still people out there with TCL modules I agree with you.

Regarding making it configurable..... I am not so sure.

Pros:

1. Flexibility.

Cons:

1. Yet another option cluttering the --help
2. More code to maintain

However, if you are using TCL modules with Lmod, these will be recognized as modules that can be loaded, which is not what you want. Any other combination (TCL modules with Tmod, or lua modules with Lmod), will result in an invisible (to the modules tool) backup. So yes, I guess I should also prepare an option for it.....

clarify portability?

clarify: the number is optional (only when existing backups are already there)

use :param: to document arguments

also support hide named argument to backup to a hidden file

I'd make this an error instead?

missing additional tests for find_backup_name_candidate and move_file?

rather than simply not removing, shouldn't we be renaming the module here already to its backup name?

this whole block is rather big to just shove it into make_module_step like this, it's better to move it into a separate method named back_up_existing_module_file or something like that?

indent is off here?

Also, why only do hidden for Tcl modules?

The backup should always be a hidden module imho, you don't want to expose the backup module to users?

Maybe we should also back up outside of the module tree, e.g. in the current directory (or at a location specified via --backup-modules?

Because lua modules have to end in .lua, otherwise they aren't treated as modules by Lmod, and don't show up with ml av. So there is no need to keep them hidden since the backup extension avoids that file ends in .lua.

Tcl modules on the other need to be hidden. Which is not that nice, since Lmod can still show them with --show-hidden. But that's the best compromise IMO. The alternative would be to remove the module header.

Backing up outside of the module tree kind of defeats the purpose of the backup. In my opinion the backup should be used for quick verification of the changes and for doing a "rollback" if necessary. If I have the current module and the backup in different files it slightly makes it more complicated and cumbersome, and more difficult to directly see if there was a previous version at any point with a simple ls

use os.path.split & os.path.join, spaces around +

use os.path.split & os.path.join, spaces around +

use write_file (it has an append mode)

Hmm, should this be a warning? It's just an informative message, no?

rather than copy-pasting all the checks, we should hoist them in a helper function, and simplify the test to

def test_backup_modules(self):
    """Test use of --backup-modules."""
    def run_checks():
        ...

    ...

    toy_mod_fn = '0.0-deps'
    run_checks()

    if isinstance(self.modtool, Lmod):
      toy_mod_fn = '0.0-deps.lua'
      run_checks()