Re: "return" should not continue script execution, even if used inappropriately

[don fong]

>
>  | the "answers" have mostly been about "how to do this".
>
> Yes, that would be the point, wouldn't it?
>

no.  a discussion thread can have multiple points.  one point that arose
was someone calling this technique (or python programmers, or both) crazy.
that's the one i'm talking about, as i've repeatedly emphasized.  i'm not
the one who introduced this point, but i thought it deserved a response
since the bash FAQ is dismissing a valuable technique, one that ought to be
more widely adopted.

Re: "return" should not continue script execution, even if used inappropriately

[don fong]

>
> This is also simple.   He's just not listening to the answers


that's not only an unwarranted personal attack, but it's untrue.

the "answers" have mostly been about "how to do this".  however as i've
stated multiple times: i'm not asking how to do it, i'm explaining why this
technique which is denigrated in the bash FAQ is a good one and should be
more widely used.  since someone dismissed it as the product of "crazy
python people", i've been trying to explain why it is not crazy, and why
the alternative solutions posted by you are less satisfactory.

and is
> insisting on doing it how some python FAQ answer says can be
> done, but which also says not to do...


as you well know it's from a bash FAQ, bashFAQ/109
 .

If the script demands args, then it can give a usage message,
> and it can be run again as a standalone command (since passing
> args to a '.' script is non standard).   Or they can just be given to
> the script as

sh -c '. scriptname' scriptname arg arg arg


that's either a terrible user experience, or requires a separate wrapper
script which would lose the advantages of a single self-contained file.








On Thu, Jan 24, 2019 at 8:24 PM Robert Elz  wrote:

> Please, both of you, stop top posting, it is anti-social, rude,
> and just lazy.
>
> If you go back to the original question (Jan 6) it was about
> a script that did "return" which did not stop the script executing
> whenencountered when the script was run as a command rather
> than via ".".
>
> It was later explained that the script in question came from
> elsewhere and could not be changed.
>
> To that the answer is simple, run it via ".".   If you don't know
> enough about the script to know how it should be run, you get
> the same answer, run it via "." (which can easily be done as
> "sh -c '. scriptname'")
>
> If the script demands args, then it can give a usage message,
> and it can be run again as a standalone command (since passing
> args to a '.' script is non standard).   Or they can just be given to
> the script as
> sh -c '. scriptname' scriptname arg arg arg
>
> There is nothing difficult about any of this.   The only issue was the
> surprise that "return" in a script that is not run via '.' and is not in
> a function does not cause the script to exit ... but that's just how
> it was originally defined, and now what happens is unspecified,
> so it simply is *never* safe to assume anything about such a thing.
>
> Don Fong's messages went off on a tangent about how a script could
> both be usable via '.' and standalone - which is a fine objective.
>
> This is also simple.   He's just not listening to the answers and is
> insisting on doing it how some python FAQ answer says can be
> done, but which also says not to do...
>
> kre
>
>
>

Re: "return" should not continue script execution, even if used inappropriately

[don fong]

>
> Encapsulate your code in a main function and deal with it.
>

yes, exactly.  and that supposedly "crazy" python convention (its bash
equivalent) is the best way i know of to "deal with it".


On Thu, Jan 24, 2019 at 4:08 PM konsolebox  wrote:

> Encapsulate your code in a main function and deal with it.
>
> -- konsolebox/m
>
> On Mon, Jan 21, 2019, 12:39 PM Robert Elz 
>> Date:Sun, 20 Jan 2019 17:43:04 -0800
>> From:don fong 
>> Message-ID:  <
>> cahqakpxao-pbdr2e0tnhh_iouhlwdxq_fccbzo1gxrhpfv9...@mail.gmail.com>
>>
>>   | i don't see how this helps.  the point is to have one file of code
>> that
>>   | behaves differently depending on whether it's dotted in or executed
>> at the
>>   | top level.
>>
>> That's fine, if you are writing something to work like this, you
>> just make sure that it will work when run either way.  That means
>> rthat you cannot do a "return" outside of a function in the script.
>>
>> This is easy to accomplish, the code just needs to be written
>> to meet both sets of requirements (if you want to finish in a reliable
>> way, you execute to EOF, and don't use either exit or return).
>>
>>   | the script should do nothing but define stuff when dotted in;
>>   | but call the main function (or some other function) when run at the
>> top
>>   | level. "sh -c '. script'" doesn't accomplish that.
>>
>> No, but it allows you to test a script that is not written to be able
>> to be run either way, someone's script that is only intended to be
>> executed as ". script" which you then want to test.In many cases
>> you might need to add more than just the '.' command - depending
>> on what is in the script (as I said way back in my first message on
>> this subject).
>>
>> kre
>>
>>
>>

Re: "return" should not continue script execution, even if used inappropriately

[don fong]

Greg Wooledge, and Bize Ma, thanks.  to be clear, i wasn't asking "how to
do it", i was just trying to explain why the supposedly "crazy" or "weird"
python convention makes a lot of sense even in the bash context.

addressing this from the FAQ: "Bash can do this, but it's not a natural
style, and you shouldn't be doing it."  it does not seem unnatural to me.
and i do think people should be doing it, if they want to write testable
bash code.


On Mon, Jan 21, 2019 at 12:47 PM Bize Ma  wrote:

> --
> *From*: Greg Wooledge
> *Subject*: Re: "return" should not continue script execution, even if used
> inappropriately
> *Date*: Mon, 21 Jan 2019 09:01:33 -0500
> ------
>
> On Sun, Jan 20, 2019 at 05:43:04PM -0800, don fong wrote:
> >* i don't see how this helps.  the point is to have one file of code that*
> >* behaves differently depending on whether it's dotted in or executed at
> the*
> >* top level.*
> https://mywiki.wooledge.org/BashFAQ/109
>
>
>
> This seems to work:
>
> [ "$BASH_SOURCE" = "$0" ] && echo "This file is meant to be sourced,
> not executed" && exit 1
> [ "$BASH_VERSION" ] || { echo "Please use bash" ; return 3; }
> [[ "${BASH_VERSINFO[0]}" -le 2 ]] && echo 'No BASH_SOURCE array
> variable' && return 2
> echo "this file seems to be sourced"
>

Re: ${p+\"$p\"}

[don fong]

here is another possible workaround, apologies if this has already been
mentioned.

cat < wrote:

> On Mon, Jan 21, 2019 at 09:14:26PM +0800, Dan Jacobson wrote:
> > So how am I to get
> > "A"
> > with bash?
> >
> > $ cat z
> > p=A
> > cat < > ${p+\"$p\"}
> > ${p+"$p"}
> > EOF
> > $ bash z
> > \"A\"
> > A
> > $ dash z
> > "A" <=WANT THIS
> > A
> > $ bash --version
> > GNU bash, version 5.0.0(1)-release...
>
> So, if I'm reading this correctly, you have a variable p whose content
> is a string ABC (without quotes).  You want a parameter expansion which
> checks whether the variable p is set, and if so, emits the string "ABC"
> (with quotes).
>
> Here you go:
>
> p=ABC
> q=\"
> printf '%s\n' "${p+$q$p$q}"
>
>

Re: "return" should not continue script execution, even if used inappropriately

[don fong]

>
> Then use the "  sh -c '. script'  " version instead.
>

i don't see how this helps.  the point is to have one file of code that
behaves differently depending on whether it's dotted in or executed at the
top level.  the script should do nothing but define stuff when dotted in;
but call the main function (or some other function) when run at the top
level. "sh -c '. script'" doesn't accomplish that.

On Thu, Jan 10, 2019 at 2:53 PM Robert Elz  wrote:

> Date:Tue, 8 Jan 2019 23:40:40 -0800
> From:don fong 
> Message-ID:  <
> cahqakpu03jfz3smu0hcfie9rh8+kyqxv5gy7h6t-7zwf-x5...@mail.gmail.com>
>
>
>   | to me, your suggested wrapper script pattern seems unnatural.  i don't
>   | always want users to have to carry around 2 files (the dottable
> library and
>   | the wrapper to dot it in).
>
> Then use the "  sh -c '. script'  " version instead.
>
>   | speaking of breakage, i'd also note that your suggested pattern has a
> bug,
>   | it assumes that run-dotscr will only be run from the directory where
> the
>   | file lives.  yes, the bug can be easily fixed.  but fixing it will make
>   | your script a bit less "simple and natural".
>
> Not really a bug, simply a simpliciation for the purposes of this e-mail
> exchange ... obviously you're expected to be smart enough to use the
> actual form that is needed to work in the environment in which the
> script is to be tested - whether that means using a full path name, or
> a PATH search, or whatever is appropriate for the situation (that is, the
> "./dotscr" part of the example should be the same thing you'd expect the
> user to type if they were to run it as an independent script.
>
> The purpose of the example was to illustrate the echnique, not to provide
> a ready made (complete) solution.
>
> kre
>
>

Re: Sourcing a script from a pipe is not reliable

[don fong]

interesting.  it looks like somehow the ". /dev/stdin yes" side isn't
waiting for the "echo echo '$1'" side to write to the pipe.

in theory, you should be able to delay the echo and it should still work...

(sleep 1; echo echo '$1') | . /dev/stdin yes

but on my mac, adding the sleep makes it fail reliably on the first
iteration.

on my linux machine, it seems to succeed reliably even with the sleep - as
expected.






On Wed, Jan 9, 2019 at 10:54 PM Jeremy  wrote:

> Configuration Information [Automatically generated, do not change]:
>
> Machine: Mac
>
> OS: Darwin
>
> Compiler: gcc
>
> Compilation CFLAGS: Xcode
>
> uname output: Darwin Octo.local 15.6.0 Darwin Kernel Version 15.6.0: Thu
> Jun 21\
>
>  20:07:40 PDT 2018; root:xnu-3248.73.11~1/RELEASE_X86_64 x86_64
>
> Machine Type: x86_64-Apple-Darwin
>
>
> Bash Version: 3.2
>
> Patch Level: 48
>
> Release Status: relase
>
>
> Although bashbug listed the Patch Level as 48 (and misspelled "release")
> the version string is
>
> 3.2.57(1)-release (x86_64-apple-darwin15)
>
>
> This is on MacOS 10.11.6. If there is a better place for me to report this
> bug, please let me know.
>
> Description:
>
> Sourcing a script from a pipe is not reliable.
>
>
> Repeat-By:
>
> This command line should run forever:
>
>
>   i=0; while [ "_$(echo echo '$1' | . /dev/stdin yes)" = "_yes" ]; \
>
>   do echo -n .; ((i++)); done; printf "\n%s\n" $i
>
>
> When I run it, it usually terminates with $i much less than 1,000.
>

Re: "return" should not continue script execution, even if used inappropriately

[don fong]

>
> df...@dfong.com said:
>   | there's a good reason for the "craziness": it enables individual
> testing of
>   | the script's functions.
>
> For that kind of use there's a trivial solution (as there often
> is for cases when people are sure that the current definition
> is inadequate).
>

to be clear, i wasn't asking how to do it, i was only explaining why the
"python craziness" is not crazy at all.


> The one piece of advice from that python related BashFAQ that Greg
> referred to which is worth following is ...
>
Bash can do this, but it's not a natural style, and you shouldn't be doing
> it.


to convincingly argue that "you shouldn't be doing this" requires more than
a raw opinion backed by nothing but a completely subjective notion of
"naturalness".

i'd note that the cited page begins this premise:
"There seem to be *two reasons* why people ask this: either they're trying
to detect user errors and provide a friendly message, or they're Python
programmers who want to use one of Python's most idiosyncratic features in
bash."

but the premise misses a powerful third reason.  i'm not a "python
programmer wanting to use an idiosyncratic feature of python", i'm a
programmer who wants to write testable code.  this so-called "unnatural"
pattern from python makes the functions within a module testable without
needing to write a wrapper script.  this is a very practical and worthwhile
advantage.

to me, your suggested wrapper script pattern seems unnatural.  i don't
always want users to have to carry around 2 files (the dottable library and
the wrapper to dot it in).  this is, again, a practical disadvantage.  it's
just one more thing to break.

speaking of breakage, i'd also note that your suggested pattern has a bug,
it assumes that run-dotscr will only be run from the directory where the
file lives.  yes, the bug can be easily fixed.  but fixing it will make
your script a bit less "simple and natural".


On Mon, Jan 7, 2019 at 5:47 PM Robert Elz  wrote:

> Date:Mon, 7 Jan 2019 08:55:58 -0500
> From:Greg Wooledge 
> Message-ID:  <20190107135558.reqhfhr5vy3ih...@eeg.ccf.org>
>
>   | https://mywiki.wooledge.org/BashFAQ/109
>
> Which only works when the shell is bash...
>
> df...@dfong.com said:
>   | there's a good reason for the "craziness": it enables individual
> testing of
>   | the script's functions.
>
> For that kind of use there's a trivial solution (as there often
> is for cases when people are sure that the current definition
> is inadequate).
>
> If you have a script "dotscr" and you want to test it, then
> you do ...
>
> cat <<-EOF >run-dotscr
> . ./dotscr
> EOF
>
> and then
>
> sh run-dotscr   # or bash, or mksh, or ...
>
> You can probably abbreviate that as
>
> sh -c '. ./dotscr'
>
> What's more, if dotscr is as most scripts designed to be used
> via the . command, and has no actual executable code (in the
> sense that it consumes no input and produces no output, so
> aside from checking for syntax errors, the above does nothing
> useful) you can add extra commands into the run-dotscr script;
> or as extra commands after a ';' in the -c case,  to actually call
> the functions dotscr defines, or the variables it creates, or
> whatever it does which needs testing.
>
> Or alternatively, interactively ...
>
> sh  # start a new shell (any appropriate shell)
> . ./dotscr
> # do whatever testing you lile
> exit
>
> Also, of course, it is also possible to write a script that can be
> executed either via the '.' command, or as a standalone script,
> if that is the desire - in fact many (perhaps most) scripts not
> expressly designed to only work as "dot scripts" are like that.
>
> The one piece of advice from that python related BashFAQ that
> Greg referred to which is worth following is ...
>
> Bash can do this, but it's not a natural style,
> and you shouldn't be doing it.
>
> kre
>
>
>
>

Re: "return" should not continue script execution, even if used inappropriately

[don fong]

>
> Crazy python people.
>

there's a good reason for the "craziness": it enables individual testing of
the script's functions.  it's a good idea, and IMHO it should be more
widely used in "production" shell scripts.

On Mon, Jan 7, 2019 at 5:56 AM Greg Wooledge  wrote:

> On Mon, Jan 07, 2019 at 04:20:46PM +0700, Robert Elz wrote:
> > Date:Sun, 6 Jan 2019 16:58:59 -0600
> > From:Dennis Williamson 
> > Message-ID:  <
> canaoh6+bmbmcblxgqug7gxfml78e9te+h8oke9thwqupyft...@mail.gmail.com>
> >
> >   | You should be able to protect yourself from this by detecting if a
> script
> >   | is not being sourced when it's intended that it must be and acting
> >   | accordingly.
> >
> > If you can work out how to do that in a portable way, and modify the
> > script to do it (and that is a reasonable thing to do) that would work.
>
> Crazy python people.
>
> https://mywiki.wooledge.org/BashFAQ/109
>
>

Re: Identical function names in bash source code

[don fong]

for my 2c, the post seems within the charter.

quoting from the bug-bash info
page:

> This list distributes, to the active maintainers of BASH (the Bourne Again
> SHell), bug reports and fixes for, and *suggestions for improvements in
> BASH. *




On Sat, Jan 5, 2019 at 9:15 AM Eduardo A. Bustamante López <
dual...@gmail.com> wrote:

> On Sat, Jan 05, 2019 at 08:19:38AM -0600, Peng Yu wrote:
> > Hi,
> >
> > It is not uncommon to see the same name is used to defined functions
> > in different .c files in bash source code.
> >
> > For example, sh_single_quote is defined in both lib/readline/shell.c
> > and lib/sh/shquote.c with the exact same signature. The two pieces of
> > code are slightly different. Do they do the exact same things or do
> > something different?
> >
> > In either case, is having the same name for different functions a good
> > practice? This will make the linked binary dependent on the order of
> > the corresponding .a files specified. Or if linked via .o files, then
> > one function will shadow the others. See 1) and 2) below for minimal
> > working examples. Neither cases seem to be good and could be avoided
> > easily by giving the functions unique names.
> >
> > So, should such functions with the same name be named differently?
> Thanks.
>
> This mailing list (bug-bash) is for reporting BUGS. If you must, at least
> please
> use help-bash for generic questions?
>
> Although you should probably be asking this to a C programming forum...
>
>

Re: Add sleep builtin

[don fong]

wouldn't it be less confusing if the proposed built-in sleep function were
given a new name instead of overloading "sleep"?


On Tue, Aug 21, 2018 at 4:34 AM, konsolebox  wrote:

> On Tue, Aug 21, 2018 at 4:23 PM Ilkka Virta  wrote:
> >
> > On 21.8. 02:35, Chet Ramey wrote:
> > > I don't think there's a problem with a `syntax conflict' as long as any
> > > builtin sleep accepts a superset of the POSIX options for sleep(1).
> >
> > The sleep in GNU coreutils accepts suffixes indicating minutes, hours
> > and days (e.g.  sleep 1.5m  or  sleep 1m 30s  for 90 seconds). I didn't
> > see support for those in konsolebox's patch, so while that's not
> > conflicting syntax per se, the lack of that option might trip someone.
>
> That was intended, and this patch is basically just a copy of the
> loadable version.  I don't really find it necessary to make the builtin
> sleep a complete copy of the external one.  The code would significantly
> increase if we add a parser for those formats.  Also it's basically
> people's fault for not reading documentation.  One should be aware
> enough if they enable the builtin.  Mksh's sleep also does the same.
>
> Anyway I respect whatever Chet decides it to become.
>
> --
> konsolebox
>
>

Re: misleading error message from variable modifier

[don fong]

will the coverage target be in an upcoming release?

On Sat, Mar 17, 2018 at 7:13 PM, Chet Ramey <chet.ra...@case.edu> wrote:

> On 3/17/18 6:39 PM, don fong wrote:
> > Chet, thanks for the tip about where to find the tests for subst.c .  i
> > still think that my tests cover some cases that aren't covered by
> > posixexp.tests .
>
> There are other test cases.
>
> > it's cool that you increased the coverage of subst.c .  how did you
> produce
> > the report?  i didn't see a script or makefile target to do it.
>
> I wrote a Makefile target to build a gcov-enabled binary and ran the
> test suite with it.
>
> --
> ``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: misleading error message from variable modifier

[don fong]

Chet, thanks for the tip about where to find the tests for subst.c .  i
still think that my tests cover some cases that aren't covered by
posixexp.tests .

it's cool that you increased the coverage of subst.c .  how did you produce
the report?  i didn't see a script or makefile target to do it.




On Wed, Mar 14, 2018 at 7:08 PM, Chet Ramey <chet.ra...@case.edu> wrote:

> On 3/9/18 3:14 AM, don fong wrote:
>
> >
> > my question was whether you have tests for the variable modifiers.
> > i don't see any.  that's the area of code i was touching, and that's why
> i
> > wrote a few tests of that area.
>
> Thank you for the inspiration. I ran the devel version of the test suite
> through gcov, added some tests, and was able to increase the coverage of
> subst.c (the word expansion code, plus) from 83% to 86%. It's tough to get
> it higher than that due to the functions that exist only for readline
> support and the debugging and system call error-handling code.
>
> 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: misleading error message from variable modifier

[don fong]

> If you'd like to augment the test suite where you feel it lacks something,
> please feel free to do so.


tests were included in my patch.  you deleted them.  i think they should
be added in.

> are there any tests that cover the variable modifiers, either unit tests
> or
> > functional tests?
> Yes. The test framework and tests are available for you to see.


my question was whether you have tests for the variable modifiers.
i don't see any.  that's the area of code i was touching, and that's why i
wrote a few tests of that area.

On Thu, Mar 8, 2018 at 7:10 AM, Chet Ramey <chet.ra...@case.edu> wrote:

> On 3/7/18 1:00 PM, don fong wrote:
> > Chet Ramey wrote:
> >> What are the most important features that you consider to lack
> unit tests?
> >
> > are there any tests that cover the variable modifiers, either unit tests
> or
> > functional tests?
>
> Yes. The test framework and tests are available for you to see. They're all
> tests of shell behavior, since what concerns most people is shell behavior.
> If you'd like to augment the test suite where you feel it lacks something,
> please feel free to do so. I'm not interested in adding a new testing
> framework.
>
> --
> ``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: misleading error message from variable modifier

[don fong]

Chet Ramey wrote:
> What are the most important features that you consider to lack unit tests?

are there any tests that cover the variable modifiers, either unit tests or
functional tests?

is any of subst.c covered by unit tests?

how do unit tests get run on this project?  i don't see a unit test target
in
the Makefile.  the "make tests" target mentioned in the INSTALL file,
seems to do only functional tests.




On Sun, Mar 4, 2018 at 1:59 PM, Chet Ramey <chet.ra...@case.edu> wrote:

> 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

[don fong]

L A Walsh wrote:

1) I see no benefit in the use of extra braces.  It diminishes
> comprehension in this case.


really?  to me the braces make the code easier to read, in the context of
the surrounding
function.  they clarify the intention.

*if (THE VALUE IS OK) {*
*USE THE VALUE*
*}*
*else {*
*EMIT AN ERROR MESSAGE*
*}*

the underlying logic is the same as the pre-existing code.  all i changed
was
the *EMIT AN ERROR MESSAGE* part.  because it then became an if-statement
in its own right, i added the surrounding braces.

in many coding standards, the braces would be required.  although i didn't
raise this as an issue, i don't think it's an improvement either.

the alterations that i question are:

* changing the variable name from check_nullness (as in the calling routine)
to check_null.

* flipping the if from positive to negative

* changing the condition from a boolean test to a comparison.

* and, outside of source code itself, dropping the tests.

Owners that require submitters to
> always use the owner's exact formatting (including tab/space
> and indentation preferences) will tend to dissuade contributions.


as a first-time submitter, i'm feeling somewhat discouraged by
alterations that i think lower the quality of the code, and especially
by the dropped tests.  i don't care so much about the formatting.


On Tue, Mar 6, 2018 at 9:51 AM, L A Walsh <b...@tlinx.org> wrote:

>
>
> don fong wrote:
>
>> my patch (form (A)):
>>
>> -report_error (_("%s: parameter null or not set"), name);
>> +{
>> +  if (check_nullness)
>> +  report_error (_("%s: parameter null or not set"), name);
>> +  else
>> +  report_error (_("%s: parameter is not set"), name);
>> +}
>>
>> the new code (form (B)):
>>
>>else if (check_null == 0)
>>  report_error (_("%s: parameter not set"), name);
>>else
>>  report_error (_("%s: parameter null or not set"), name);
>>
>>
> 1) I see no benefit in the use of extra braces.  It diminishes
> comprehension in this case.
>
> 2) The purpose appears to, optionally treat a null value for ptr name
> as either an "unset" condition (as in never set) vs. mentioning
> that it might also be the case that it might have been set, but with
> a null value.
>
> OTOH, if the purpose is to vary the error message, I might find
> 1 call to be more clear:
>
>report_error( check_null ? _("%s:  parameter null or not set")
> : _("%s:  parameter not set"),name );
>
> Whether or not 'name )' would be on a separate line, or set off
> with extra spaces would depend on code width compared with surrounding
> lines.  That also assumes whether or not the macro '_()' can be
> used more than once within the call to report_error.  It might be
> that the format above would exceed some desired code width, which
> might "recommend" a different formatting.
>
> It's hard to say w/o knowing other conventions in the code.
>
> However, personally, I'd find the fact that it was accepted and
> simply adapted to whatever the code owner wanted in this situation, an
> overriding benefit.  Owners that require submitters to
> always use the owner's exact formatting (including tab/space
> and indentation preferences) will tend to dissuade contributions.
>
> I have things that are more clear for me to read and comprehend,
> while others have their own "input format" that benefit them as well.
> Ideally, computers can be used to automatically reformat such
> differences as code is imported or exported.
> -l
>
>
>
>
>

Re: coding standards

[don fong]

Clark,

Just took a look at the code and it is an int:


declaring boolean quantities as int is a common practice in old C code.
indeed, all the boolean vars in this program seem to be declared as int.
at least, i don't see anything declared as bool.

declared type notwithstanding, in the context of subst.c, check_nullness
is being used as a boolean.  unfortunately, in retrospect the snippets i
posted are not as clear as i had thought.  (is there a way to provide a URL
for subst.c that links back to the devel branch at git.savannah.gnu.org?
as can be done with github or gitlab.)

consider:

* there is only one line where check_nullness is set to a non-zero value.
* that line is executed at most once.
* it does check_nullness++ which is another common practice for setting
  a boolean variable in C code.

 8449   check_nullness++;

* there are at least 2 other references to check_nullness in a boolean
context:

 8689   if (check_nullness)

 8687   var_is_null = check_nullness && (var_is_set == 0 || *temp == 0);

while these boolean-style references may not prove that check_nullness
is boolean, it at least shows that using it as a boolean is not inconsistent
with existing practices.

if you agree it's a boolean, wouldn't it be better to test its truth/falsity
directly instead of comparing it to false (0)?  the latter seems like a
typical "trap" that coding pundits warn against.

consistency would also argue for using the same variable name,
check_nullness
(as in my patch) instead of check_null (as in the altered version).










On Sun, Mar 4, 2018 at 6:15 PM, Clark Wang <dearv...@gmail.com> wrote:

> On Mon, Mar 5, 2018 at 9:13 AM, don fong <df...@dfong.com> 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;
>
>

Re: coding standards

[don fong]

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 <dearv...@gmail.com> wrote:

> On Sun, Mar 4, 2018 at 5:15 AM, don fong <df...@dfong.com> 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.
>

coding standards

[don fong]

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?

NOTE: i'm OK with the fact that alterations were made.  but i wonder what
was the reasoning?  as near as i can tell, my patch could have been just
applied verbatim, but it wasn't.  and i don't see how the alterations were
an improvement.

all of which makes me wonder what is the review process for changes to the
bash source?

here is the actual code from which the above example was derived, in
subst.c.

my patch (form (A)):

-report_error (_("%s: parameter null or not set"), name);
+{
+  if (check_nullness)
+  report_error (_("%s: parameter null or not set"), name);
+  else
+  report_error (_("%s: parameter is not set"), name);
+}

the new code (form (B)):

   else if (check_null == 0)
 report_error (_("%s: parameter not set"), name);
   else
 report_error (_("%s: parameter null or not set"), name);

a couple of other minor alterations were made.

* my code used the same variable name check_nullness which was used in the
calling routine, vs check_null in the altered version.  i'm OK with it even
though i think using the same variable name to stand for the same thing
would be slightly better.

* my code's error message was a sentence, "parameter *is* not set" vs the
terse "parameter not set".  there is a certain consistency to omitting the
verb, if you don't care about clarity.  i'm OK with this.

Re: Unset array doesn't work

[don fong]

Robert Elz:

> And yet when that change to the entrenched behaviour was made,
> there were no complaints?   And there's no option to switch back to
> the previous way?   Kind of suggests just how important everyone
> believes the original method was, doesn't it?


doesn't the same argument apply even more strongly to your proposed
change?  from the fact that the "bug" has been in existence for decades
without a huge clamor for it to be "fixed", your own reasoning would imply
that "fixing" it can't be that important.

Koichi Murase:

> I don't agree with changing the default behavior that removes the
> placeholder of previous-context variables. The reason is just backward
> compatibility, but it's important.


speaking as a mere bash user, i want to give a huge +1 to Koichi's point.
after decades of bash doing unset one way, changing the behavior carries a
risk
of breaking existing scripts.

the upside seems small considering that if people aren't even aware of this
behavior, it can't be much of a pain point now can it?  but it can become a
pain point if bash changes something that people have been relying on,
even unknowingly.

IMHO, testability should be a primary factor when considering any new bash
features.  from a testing standpoint there are "too many" shell options
already.
and it's not just about testing bash itself.  how many existing shell
scripts are
going to break because some user unknowingly ran the script with a
combination of options that the script author wasn't aware of?

so even if it were easy to implement as a shell option, it doesn't sound
like a good idea to me.  i might feel differently if bash had comprehensive
test coverage.  AFAIK the tests are nowhere near strong enough.

changing the default setting of the proposed option seems even more risky.


On Fri, Mar 2, 2018 at 11:23 PM, 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:
>
>   | 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.
>
>   | This is how bash dynamic scoping works. The exception for the
> declaration/
>   | unset at the current scope was added 16 years ago, and the existing
>   | behavior was already entrenched.
>
> And yet when that change to the entrenched behaviour was made,
> there were no complaints?   And there's no option to switch back to
> the previous way?   Kind of suggests just how important everyone
> believes the original method was, doesn't it?
>
>   | 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).   I know that's the opposite of what would
> usually be done in order to retain backwards compat, but for this,
> I think it would be a useful test to see if anyone notices the difference.
> You can always change it for beta/final releases if there are issues.
> If not, perhaps the option can just go away (then or later.)
>
>   | > Lastly, where does the notion of "remove" come from?
>   |
>   | As a way to describe the historical bash behavior, it works.
>
> 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.
>
> The converse is correct - a variable that does not exist appears as
> an unset variable when referenced.
>
> There are (even ignoring the unset command) too many ways
> (in bash, as well as other shells) to get variables that patently
> obviously "exist" in some form or other but are unset.
>
> The most obvious example is
>
> export newvar
>
> after that
>
> echo ${newvar-unset}
>
> prints "unset".   Sometime later if we give newvar a value, it, and its
> new value are exported - demonstrating that the export attribute was
> remembered (ie: "newvar" existed before it was set - it must have done
> in order to retain an atttribute).
>
> jinx$ export newvar
> jinx$ echo ${newvar-unset}
> unset
> jinx$ newvar=set
> jinx$ printenv newvar
> set
> jinx$ echo $BASH_VERSION
> 

Re: misleading error message from variable modifier

[don fong]

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?

on general principles, when someone makes a change, there ought to be a
test to show that it does what it's supposed to.  that's just good
engineering.  there also ought to be sufficient pre-existing tests to show
that the change didn't break anything else.  that goes double when patches
are being submitted by outsiders and newcomers (like me).

before i submitted my patch, i looked around in the test directory to see
if there were any tests that covered the behavior i was changing.  i didn't
see any tests of the variable modifiers at all.  that struck me as a
situation that should be changed.  so i added some tests to cover my own
change plus a bit more.

as far as i can tell, the vast majority of the C code here seems to lack
unit tests as well.

for a widely used program like bash, IMHO it'd be worthwhile to have
automated tests (either unit tests or functional tests) that cover every
documented feature.

if that hasn't been the case in the past, i propose that it be adopted as a
new goal.  there should at least be a rule that all new code should be
covered by tests.  i'm astonished to learn that the tests that i
contributed were simply discarded.






On Fri, Mar 2, 2018 at 12:21 PM, Chet Ramey <chet.ra...@case.edu> wrote:

> On 3/1/18 11:37 PM, don fong wrote:
> > Chet, thanks.  in subst.c there is code that looks similar to what i had
> > suggested.  but i don't see the tests that i submitted.  i also don't see
> > the change listed in CHANGES?
>
> I didn't think the tests were necessary. And the ChangeLog file is the
> one to look at (eventually resolves to CWRU/CWRU.chlog, as Clark noted).
> The CHANGES file is updated as part of release engineering; as you might
> suspect, I'm working on bash-5.0-alpha.
>
> 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: misleading error message from variable modifier

[don fong]

Chet, thanks.  in subst.c there is code that looks similar to what i had
suggested.  but i don't see the tests that i submitted.  i also don't see
the change listed in CHANGES?







On Thu, Mar 1, 2018 at 11:09 AM, Chet Ramey <chet.ra...@case.edu> wrote:

> On 3/1/18 12:24 PM, don fong wrote:
> > any feedback on this patch?
>
> Check the current devel git branch.
>
> --
> ``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: misleading error message from variable modifier

[don fong]

any feedback on this patch?

On Sat, Feb 24, 2018 at 11:24 AM, Chet Ramey <chet.ra...@case.edu> wrote:

> On 2/23/18 10:20 PM, don fong wrote:
> > hi folks.  i'm a bash user, who just noticed a slight anomaly.  it has
> > to with the shell variable modifier ${parameter?} .  according to the
> > man page, ${X?} should yield an error message and exit if X is unset,
> > otherwise the value of X.  in this case,
> >
> > unset X; echo ${X?}
> >
> > i expect to get an error message, and indeed an error message results.
> >
> > bash: X: parameter null or not set
> >
> > but i think the error message is misleading.
>
> Thanks for the report and patch. I'll take a look.
>
> 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: Unset array doesn't work

[don fong]

seems like it would be helpful to explain this behavior more clearly in the
man page.




On Wed, Feb 28, 2018 at 12:14 PM, Chet Ramey  wrote:

> On 2/28/18 3:00 PM, Greg Wooledge wrote:
>
> > Does unset create some kind of "placeholder" in the current function
> > (but not in a caller)?
>
> Yes, that's what I said. In the current scope, unset arranges for the
> variable to appear unset. In a previous scope, unset just removes the
> variable, which uncovers an instance of the variable at a (further)
> previous scope.
>
> It looks like I added that code in 1995. The code before that was "pure"
> dynamic scoping, in the sense that it just removed the variable and
> `uncovered' a previous scope's value no matter where the variable was
> declared. It seems like I added the special case for several reasons,
> but there's no indication of widespread user complaint about the behavior
> of `unset'. (Of course, that was a long time ago.)
>
> --
> ``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: add generated files to .gitignore

[don fong]

Chet, thanks for the suggestion.

i still wonder what's the objection to changing .gitignore?

using a separate directory to build, while i'm working on the sources,
feels less convenient.





On Sun, Feb 25, 2018 at 1:10 PM, Chet Ramey <chet.ra...@case.edu> wrote:

> On 2/25/18 2:49 PM, don fong wrote:
> > Chet, i'm not sure i understand your suggestion.
> >
> >> You don't have to build in the source directory.
> >
> > i don't see anything in the INSTALL or README files about building
> outside
> > the source dir.
> > according to INSTALL,
>
> This is a standard feature of any autoconf-generated configure script, and
> is not specific to bash. Make a directory, cd to it, run
> "bash /path/to/srcdir/configure" and configure will make sure the right
> paths end up in the generated Makefiles.
>
> --
> ``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: add generated files to .gitignore

[don fong]

Chet, i'm not sure i understand your suggestion.

> You don't have to build in the source directory.

i don't see anything in the INSTALL or README files about building outside
the source dir.
according to INSTALL,

The simplest way to compile Bash is:
>   1. 'cd' to the directory containing the source code and type
>  './configure' to configure Bash for your system.


it sounds like you're suggesting to make a copy of the source directory?
(or if that's not what you mean, can you be more explicit?)

making a copy of the source directory has drawbacks.  i also don't see how
it solves the problem of "git status" being cluttered.  either the copy is
a git repo, in which case "git status" will still be cluttered with
generated files; or it's not a git repo in which case i won't be able to do
"git status" or "git diff" to create the patch.

either way, if i'm trying to create a patch, and i have to move my changed
files from the copy back to the original source dir, it'd be easy to
accidentally leave something out.

it seems to me that building in the source directory is the natural and
most convenient way to do things.  from what i've seen, it's also the way
that "most" other open source software works.  my proposed change to
.gitignore will facilitate that mode of operation.

what do you perceive as the drawback?






On Sat, Feb 24, 2018 at 3:29 PM, Chet Ramey <chet.ra...@case.edu> wrote:

> On 2/24/18 3:36 PM, don fong wrote:
> > Eric, thanks for the tip.
> >
> > my feeling is that regardless of whether these files are pushed, they
> > clutter up the "git status" listing after i've done a build.
>
> You don't have to build in the source directory.
>
> --
> ``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: add generated files to .gitignore

[don fong]

Eric, thanks for the tip.

my feeling is that regardless of whether these files are pushed, they
clutter up the "git status" listing after i've done a build.



On Sat, Feb 24, 2018 at 11:28 AM, Eric Blake <ebl...@redhat.com> wrote:
> On 02/24/2018 01:26 PM, Chet Ramey wrote:
>>
>> On 2/24/18 1:46 AM, don fong wrote:
>>>
>>> based on my experience creating one patch, running "make" and "make
>>> test", i found that "git status" was reporting a lot of generated and
>>> built files that i think should be ignored.
>>
>>
>> Those files aren't ever pushed to the bash git repositories (master,
>> devel).
>
>
> If Chet doesn't want to patch the primary bash.git to ignore them for
> everybody in .gitignore, you can still patch your downstream repo to ignore
> them locally by instead adding those exclusions to .git/info/exclude.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

add generated files to .gitignore

[don fong]

based on my experience creating one patch, running "make" and "make
test", i found that "git status" was reporting a lot of generated and
built files that i think should be ignored.

i added the "untracked" files to .gitignore, and this is the patch.


ign.patch
Description: Binary data

Re: misleading error message from variable modifier

[don fong]

Clark, thanks for your response.  i'm attaching my patch.


On Fri, Feb 23, 2018 at 7:32 PM, Clark Wang <dearv...@gmail.com> wrote:
> On Sat, Feb 24, 2018 at 11:20 AM, don fong <df...@dfong.com> wrote:
>>
>>
>> i would like to submit this change for inclusion in bash.  how should i
>> proceed?
>
>
> It's very common to send patches directly to this mailing list. I believe
> it's also OK to send only to Chet. :)


myfix.patch
Description: Binary data

misleading error message from variable modifier

[don fong]

hi folks.  i'm a bash user, who just noticed a slight anomaly.  it has
to with the shell variable modifier ${parameter?} .  according to the
man page, ${X?} should yield an error message and exit if X is unset,
otherwise the value of X.  in this case,

unset X; echo ${X?}

i expect to get an error message, and indeed an error message results.

bash: X: parameter null or not set

but i think the error message is misleading.  the wording sort of
implies that null would be an error, even though with the ? modifier,
null is OK.  it's kind of confusing as well, to see the same error
message from both ${X?} and ${X:?} when they are really checking
distinct things.

i think the error message should say

bash: X: parameter is not set

a co-worker challenged me to submit a patch.  so i downloaded the
code, looked at it, made a fix, and created a test script compatible
with the existing tests/ framework.  all tests pass.  (i glanced
around in the tests/ directory, did not see any tests of the variable
modifiers, so i extended my tests to cover some of the other modifiers
as well.)

i would like to submit this change for inclusion in bash.  how should i proceed?