Re: coding standards
On Sun, Mar 4, 2018 at 5:15 AM, don fong wrote: > admittedly this is a very minor point, but i am curious. this has to do > with coding standards for bash source. > > consider an if statement in C (or bash, for that matter). which is form is > better? > > Form (A): > > if (flag) > X(); > else > Y(); > > Form (B): > > if (flag == 0) > Y(); > else > X(); > > they are functionally equivalent. but IMHO (A) is slightly more readable. > first because flag (in this case) is intended to be a boolean value not > arithmetic, and second because it's simpler to think about an if when the > condition is positive. > > this is what i'd say if (B) were under code review. > > i submitted a patch with code in form (A). it was added to the code base > in form (B). was there a good reason for this mutation? > I believe the main reason is to keep consistent with existing code. I used to use ``if (flag)'' but these days I'd prefer ``if (flag != /* or == */ 0)'' which is explicit that ``flag'' is not a boolean var. I use ``if (flag)'' only when `flag' is a boolean. It's similar I used to write ``if (ptr)'' but now I prefer ``if (ptr != NULL)'' which is explicit that `ptr' is a pointer.
Re: misleading error message from variable modifier
On 3/2/18 4:45 PM, don fong wrote: > Chet, thanks for the explanation about CHANGES. i am not familiar with the > bash release process. > > as for this: > >> I didn't think the tests were necessary. > > what standard are you using to judge whether tests are necessary? does the > bash project have any coverage metrics? It's admittedly subjective, but the bash test suite is very large and concentrates on testing code that has undergone more changes than this, and is more heavily used. In this case, I didn't think a fix that added two lines of code to a relatively stable function needed a test to ensure that it doesn't change. > as far as i can tell, the vast majority of the C code here seems to lack > unit tests as well. What are the most important features that you consider to lack unit tests? -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: coding standards
On 3/4/18 7:43 AM, Clark Wang wrote: >> i submitted a patch with code in form (A). it was added to the code base >> in form (B). was there a good reason for this mutation? >> > > I believe the main reason is to keep consistent with existing code. That, plus the final change was smaller and simpler than the submitted patch. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: Unset array doesn't work
On 3/3/18 2:23 AM, Robert Elz wrote: > Date:Fri, 2 Mar 2018 14:43:02 -0500 > From:Chet Ramey > Message-ID: <4502a0e5-0600-d294-9af2-4e9eeb0a0...@case.edu> > > My final comments on this subject: Mine, too. > > | Perhaps. But bash has never done this. Not from day one. That's 30 years. > > That a bug (be it a design bug, or a coding bug) has existed a long tiime > does not make it any less a bug. > > I have been using bash for essentially all that time (from before you took > over maintainership) and I never knew it worked like that. From comments > here (where some people far more knowledgable about bash and its > internals than I are to be found) I suspect that very few other people know > about it either. There are actually a number of large projects that depend on it, most notably bash-completion. The `upvar' `package' also exploits the current behavior. > | I can see doing this and allowing it to be toggled by a shell option. > > A suggestion: Do that for bash 5, and in the alpha release, make > the option default to cause things to work the opposite way than > happens now (so the option needs to be explicitly changed to get > the current behaviour). No, I'm uninterested in the significant potential to just break bash-completion. I haven't combed my way through all that code, but I've been assured that it depends on the current behavior in some way. I may turn this hypothetical new variable on myself and see what happens with bash-completion on a system that has it installed. > Yes, that I understand. My issue is that I believe this is colouring > your thoughts on just what "unset" is - same as the "appear/be" > (trivial seeming) semantic issue you commented on in another message. > > That is, it appears to me as if you believe that "unset" (as a state, not > the command here) implies "non-existing". That's never been correct. OK, let's say a variable is an object with a name, attributes, and a value, and that being `unset' means it does not have a value. That breaks down fairly quickly: a=4 export a unset a a=7 printenv a doesn't ever print `7'. The `unset' builtin has removed the variable: there is no name, there are no attributes, and there is no value. The export attribute was not retained when a variable with the same name was given a new value. It means you can create an object with two or more of the three, and the variable will be `unset' (as a state) until it's been given a value, but `unset' (the builtin) removes all three. So the `unset' builtin removes a variable, at least at the global scope. What that means in terms of a local variable is the question. Chet -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
unexpected behavior with read
Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: darwin16.7.0
Compiler: clang
Compilation CFLAGS: -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64'
-DCONF_OSTYPE='darwin16.7.0' -DCONF_MACHTYPE='x86_64-apple-darwin16.7.0'
-DCONF_VENDOR='apple'
-DLOCALEDIR='/usr/local/Cellar/bash/4.4.19/share/locale' -DPACKAGE='bash'
-DSHELL -DHAVE_CONFIG_H -DMACOSX -I. -I. -I./include -I./lib
-I./lib/intl -I/private/tmp/bash-20180209-55597-111ek7c/bash-4.4/lib/intl
-DSSH_SOURCE_BASHRC -Wno-parentheses -Wno-format-security
uname output: Darwin Row-Town-Dander-Fleece.local 16.7.0 Darwin Kernel
Version 16.7.0: Thu Jan 11 22:59:40 PST 2018;
root:xnu-3789.73.8~1/RELEASE_X86_64 x86_64
Machine Type: x86_64-apple-darwin16.7.0
Bash Version: 4.4
Patch Level: 19
Release Status: release
Description:
global variable assignments fail when part of a sequence that
includes read and begins with a function invoked by command substitution
read can be invoked by another function, or invoked by another
command and the result is the same
Repeat-By:
"""
#!/bin/bash
r=
f ()
{
if [ -z "$r" ]; then
r="x"
read
fi
echo "$r"
}
echo "$(f)"
echo "$(f)"
"""
Re: coding standards
Clark, thanks for your answer.
I use ``if (flag)'' only when `flag' is a boolean.
but in this case, it *is* a boolean, as i stated, and as can be seen in
subst.c:
+{
+ if (check_nullness)
+ report_error (_("%s: parameter null or not set"), name);
+ else
+ report_error (_("%s: parameter is not set"), name);
+}
On Sun, Mar 4, 2018 at 4:43 AM, Clark Wang wrote:
> On Sun, Mar 4, 2018 at 5:15 AM, don fong wrote:
>
>> admittedly this is a very minor point, but i am curious. this has to do
>> with coding standards for bash source.
>>
>> consider an if statement in C (or bash, for that matter). which is form
>> is
>> better?
>>
>> Form (A):
>>
>> if (flag)
>> X();
>> else
>> Y();
>>
>> Form (B):
>>
>> if (flag == 0)
>> Y();
>> else
>> X();
>>
>> they are functionally equivalent. but IMHO (A) is slightly more readable.
>> first because flag (in this case) is intended to be a boolean value not
>> arithmetic, and second because it's simpler to think about an if when the
>> condition is positive.
>>
>> this is what i'd say if (B) were under code review.
>>
>> i submitted a patch with code in form (A). it was added to the code base
>> in form (B). was there a good reason for this mutation?
>>
>
> I believe the main reason is to keep consistent with existing code.
>
> I used to use ``if (flag)'' but these days I'd prefer ``if (flag != /* or
> == */ 0)'' which is explicit that ``flag'' is not a boolean var. I use
> ``if (flag)'' only when `flag' is a boolean. It's similar I used to write
> ``if (ptr)'' but now I prefer ``if (ptr != NULL)'' which is explicit that
> `ptr' is a pointer.
>
Re: unexpected behavior with read
On 3/4/18 6:15 PM, Zach Hadgraft wrote:
> Bash Version: 4.4
> Patch Level: 19
> Release Status: release
>
> Description:
> global variable assignments fail when part of a sequence that
> includes read and begins with a function invoked by command substitution
> read can be invoked by another function, or invoked by another
> command and the result is the same
>
> Repeat-By:
>
> """
> #!/bin/bash
>
>
> r=
>
> f ()
> {
> if [ -z "$r" ]; then
> r="x"
> read
> fi
> echo "$r"
> }
>
> echo "$(f)"
> echo "$(f)"
> """
What do you expect to happen, and what happens instead? When I run this
script, the `read' gets satisfied when you enter a line of text followed
by a newline, and `x' is assigned to r:
line
x
line
x
Since command substitution happens in a subshell, and subshells can't
affect their parent's environment, `r' isn't going to change in the
calling shell.
Chet
--
``The lyf so short, the craft so long to lerne.'' - Chaucer
``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: coding standards
On Mon, Mar 5, 2018 at 9:13 AM, don fong wrote:
> Clark, thanks for your answer.
>
> I use ``if (flag)'' only when `flag' is a boolean.
>
>
> but in this case, it *is* a boolean, as i stated, and as can be seen in
> subst.c:
>
> +{
> + if (check_nullness)
> + report_error (_("%s: parameter null or not set"), name);
> + else
> + report_error (_("%s: parameter is not set"), name);
> +}
>
Just took a look at the code and it is an int:
@@ -6849,8 +6849,9 @@ parameter_brace_expand_rhs (name, value, c, quoted,
pflags, qdollaratp, hasdolla
used as the error message to print, otherwise a standard message is
printed. */
static void
-parameter_brace_expand_error (name, value)
+parameter_brace_expand_error (name, value, check_nullness)
char *name, *value;
+ int check_nullness;
{
WORD_LIST *l;
char *temp;
