GNU bug report logs - #58812
[PATCH] Add '--symlink' to 'guix shell'

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 59164 <at> debbugs.gnu.org, 58812 <at> debbugs.gnu.org
Subject: [bug#58812] Coding style: similarly-named variables
Date: Thu, 17 Nov 2022 18:37:39 +0100

Hi,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>>
>>> * gnu/build/install.scm (evaluate-populate-directive): By default, error when
>>> the target of a symlink doesn't exist.  Always ensure TARGET ends with "/".
>>> (populate-root-file-system): Call evaluate-populate-directive with
>>>  #:error-on-dangling-symlink #t and add comment.
>>
>> [...]
>>
>>> +  (define target* (if (string-suffix? "/" target)
>>> +                      target
>>> +                      (string-append target "/")))
>>
>> Maybe make it:
>>
>>   (let ((target (if …)))
>>     …)
>>
>> so there’s only one ‘target’ in scope (and no ‘target*’); otherwise it’s
>> easy to forget the ‘*’ and refer to wrong one.
>
> It's a pattern I've used at other places; I find it more hygienic to not
> shadow existing variables; it signal to the reader "be careful, this is
> not the same as the argument-bound one, though they are closely
> related".

I don’t buy it.  :-)  The reader might be careful yet end up using the
“wrong” variable.  As long as the “wrong” variable has no use, I think
it’s best to shadow it so that mistakes cannot happen.

Of course the details vary depending on context, but I think we should
not start introducing this pattern in different places.  Perhaps
something to discuss and codify under “Formatting Code”?

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.