WIP patch to reduce rewrite_filename duplication

[Collin Funk]

Hi Bruno,

I haven't pushed this patch yet, but it goes in the correct direction,
in my opinion.

Here we were talking about the rewrite_filename functions:

     https://lists.gnu.org/archive/html/bug-gnulib/2024-04/msg00268.html

I asked about the 'test=lib/' filename replacement under mode ==
'copy-file' and you said:

> 'tests=lib/' should not occur in the scope of mode == 'copy-file'.
> But this string is an indicator inside gnulib-tool; users of gnulib-tool
> should not be allowed to pass it.

Does it make more sense to check for a filename starting with
'tests=lib/' and error out or is it safe to assume the input is clean?
Ideally I would like to use a module-level function or @staticmethod
there. Less repeated code to maintain.

The second thing I am not 100% satisfied with is the creation of
GLFileTable objects. In GLImport.prepare(), for example, it takes
three lines. The setters also take two arguments which feels a bit
unorthodox to me. After this patch in GLImport.prepare():

    # Construct the file table.
    filetable = GLFileTable(sorted(set(filelist)))
    filetable.set_old_files(self.cache, old_files)
    filetable.set_new_files(self.config, new_files)

The reason I did this is that, from what I understand, --import and
--create-testdir (and their respective classes) are essentially the
same for the most part. The main difference here is that
--create-testdir doesn't require reading a gnulib-cache.m4 file.
Therefore, there is no need for filetable.old_files. After this patch
in GLTestDir.execute():

    # Setup the file table.
    filetable = GLFileTable(filelist)
    filetable.set_new_files(self.config, filelist)

I feel that handling all different combinations for
GLFileTable.__init__() would get annoying. As an example, what is the
correct behavior when old_files is passed but a GLConfig with
directories read from gnulib-cache.m4 is not? That seems like invalid
input to me.

I feel like two @classmethod constructors would be a fine solution. Or
maybe inheritance would be better? To illustrate here is GLFileTable's
instance variables and their usage:

    class GLFileTable:
        # Used in GLImport & GLTestDir.
        all_files: list[str]
        new_files: list[tuple[str, str]]

        # Used in GLImport only.
        old_files: list[tuple[str, str]]
        added_files: list[str]
        removed_files: list[str]

Any opinions on these ideas? If not, I will probably sleep on it and
push an improved patch.

Collin

0001-gnulib-tool.py-Reduce-code-duplication-in-filename-t.patch
Description: Text Data