Re: gnulib-tool.py: Fix removal of variables from GLMakefileTable.

[Collin Funk]

Hi Bruno,

On 3/29/24 4:16 AM, Bruno Haible wrote:
>> I was not a fan of adding another value to the left side of this
>> assignment:
>>
>>     emit, uses_subdirs = self.emitter.lib_Makefile_am(basename, ...)
> 
> Why not? This is perfectly normal functional programming style.
> Lisp has it ("multiple values"), ISO C has it ("struct" as return value),
> Python has it ("tuplie"), ...

Probably mostly habit/personal preference now that I think about it. I
can only read very simple Lisp code, sadly. I would caveat the C
struct comparison a bit. One might go another direction with
incomplete types, function pointers, etc. Though I'd imagine they
would prefer C++ in that case. :)

I likely got annoyed by some terrible code I wrote a long time ago.
You have to be a bit careful with unpacking tuples:

   (a, b) = (1, 2, 3)
   Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: too many values to unpack (expected 2)
    (a, b, _) = (1, 2, 3)
    print(f'{a} {b}')
    1 2

That wouldn't be a problem in that case, but something to be careful
about.

> For example, here: If current_edit is 1, and we remove the element
> at index 1:
> 
>>>> l = ['a','b','c']
>>>> l.pop(1)
> 'b'
>>>> l
> ['a', 'c']
> 
> we need to continue at index 1, not index 2. Otherwise we miss the 'c'.

Oops, yes you are right. Thanks.

> Also, the
>    if dictionary['var']:
> lines that were meant to detect already-handled entries can now be dropped.
> 
> But I kept wondering why the original code is not working. The reason is that
> each makefiletable[index] access creates a clone of the dict object, and thus
>    dictionary.pop('var')
> has an effect on the dictionary variable but not on the makefiletable.

Interesting. I thought it was just pointless, so I didn't bother
changing it. Sort of like a pointless C cast or something...

I have been wanting to change empty list initialization from 'list()'
to '[]' since we have both at the moment. Also some lines like this
confuse the LSP I use:

    old_table += [tuple([dest, src])]
    # and
    result = tuple([filetable, transformers])

It can't tell if those lines are a tuple or a list...
Maybe that would be a good opportunity to find more bugs like that.

> Once I fix this, I get a KeyError from
>    if dictionary['var']:
> indicating that this code never worked.

Yes, that code has been around for a long time. I didn't change it
because I didn't notice any errors from it.

Your method would be correct. You could also write something like:

     if dictionary.get('var', 'default-value'):

which might make sense in other situations. Thanks for fixing that.

Collin