Re: [PATCH] gnulib-tool.py: Don't use --[no]-vc-files unless explicitly given.

[Bruno Haible]

Collin Funk wrote:
> I've attached an updated patch.

Thanks! Applied.

Bruno

Re: [PATCH] gnulib-tool.py: Don't use --[no]-vc-files unless explicitly given.

[Collin Funk]

On 3/23/24 4:49 AM, Bruno Haible wrote:
> If the variable has 3 possible values (None, False, True), then writing
> 'if value == True:' is the simpler way. Why would one use a conversion to
> bool, i.e. 3-values to 2-values conversion? It would only make things
> more complicated.

Yes, I agree now. I think that I saw True / False and figured it
should just be treated as a bool. However, the way GLConfig is set up,
vc_files along with many other variables can also be None. Therefore,
that assumption is bad. I will be more careful about that in the
future...

Type hints in the future will help:

# This
vc_files: bool | None
# Not this
vc_files: bool

I've attached an updated patch.

CollinFrom 25173b648f1c0d89cc678fae240dbdb60d67e1ff Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Sat, 23 Mar 2024 12:55:58 -0700
Subject: [PATCH] gnulib-tool.py: Don't print Python bools in gnulib-cache.m4

* pygnulib/GLImport.py (GLImport.gnulib_cache): Convert Python bools to
lowercase before printing.
---
 ChangeLog| 6 ++
 pygnulib/GLImport.py | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index cbda2c57d0..cb6e41f2cc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2024-03-23  Collin Funk  
+
+	gnulib-tool.py: Don't print Python bools in gnulib-cache.m4
+	* pygnulib/GLImport.py (GLImport.gnulib_cache): Convert Python bools to
+	lowercase before printing.
+
 2024-03-23  Bruno Haible  
 
 	gnulib-tool.py: Don't unnecessarily run configure && make in testdirs.
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 8a0021d8dc..85ec0a61db 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -601,7 +601,8 @@ class GLImport(object):
 emit += 'gl_PO_DOMAIN([%s])\n' % podomain
 emit += 'gl_WITNESS_C_MACRO([%s])\n' % witness_c_macro
 if vc_files != None:
-emit += 'gl_VC_FILES([%s])\n' % vc_files
+# Convert Python bools to shell (True -> true).
+emit += 'gl_VC_FILES([%s])\n' % str(vc_files).lower()
 return constants.nlconvert(emit)
 
 def gnulib_comp(self, filetable, gentests):
-- 
2.44.0

Re: [PATCH] gnulib-tool.py: Don't use --[no]-vc-files unless explicitly given.

[Bruno Haible]

Collin Funk wrote:
> I'm used to writing this:
> 
> if value:
> print('1')
> else:
> print('2')
> 
> instead of this:
> 
> if value == True:
> print('1')
> elif value == False:
> print('2')

If the variable has 2 possible values, then writing 'if value:' is the
simpler way of writing things.

If the variable has 3 possible values (None, False, True), then writing
'if value == True:' is the simpler way. Why would one use a conversion to
bool, i.e. 3-values to 2-values conversion? It would only make things
more complicated.

Bruno

Re: [PATCH] gnulib-tool.py: Don't use --[no]-vc-files unless explicitly given.

[Collin Funk]

On 3/23/24 3:54 AM, Bruno Haible wrote:
> But this part of your patch is a no-op, since
> 
>   >>> None == True
>   False
>   >>> None == False
>   False
> 
> No?

Oops... Yes you are right.

I'm used to writing this:

if value:
print('1')
else:
print('2')

instead of this:

if value == True:
print('1')
elif value == False:
print('2')

In the first case the check for None matters:

if None:
print('1')
else:
print('2')

prints 2.

Feel free to pick whichever style you prefer, and I'll remember too
look out for that in the future.

Collin

Re: [PATCH] gnulib-tool.py: Don't use --[no]-vc-files unless explicitly given.

[Bruno Haible]

Hi Collin,

> This patch handles some issues with --vc-files/--no--vc-files.
> gnulib-tool.py would always print this in the actioncmd message at the
> top of files. The proper behavior would be to check if vc_files ==
> None before treating it as a bool.

But this part of your patch is a no-op, since

  >>> None == True
  False
  >>> None == False
  False

No?

The second part of the patch looks OK.

Bruno