zsh-workers
 help / color / mirror / code / Atom feed
From: Martijn Dekker <martijn@inlv.org>
To: Bart Schaefer <schaefer@brasslantern.com>
Cc: zsh-workers@zsh.org
Subject: Re: [PATCH] posix_builtins: allow exporting a reaonly
Date: Sat, 21 Apr 2018 14:57:40 +0200	[thread overview]
Message-ID: <88bb2098-0b83-a320-d4b9-44ef6143ea13@inlv.org> (raw)
In-Reply-To: <CAH+w=7YJqzWaH+6dGpuZMT32ViELW0reTjNqTfPQxPfiuJip9w@mail.gmail.com>

Op 18-04-18 om 22:07 schreef Bart Schaefer:
> On Wed, Apr 18, 2018 at 12:58 PM, Martijn Dekker <martijn@inlv.org> wrote:
>> POSIX_BUILTINS incorrectly prohibits exporting a readonly variable. All
>> other POSIX shells allow this and there is nothing in the POSIX text[*] that
>> says it's not allowed. The attached patch fixes this.
> 
> I hope it's really that simple -- see thread about typeset -T and
> read-only and resulting confusion, e.g. zsh-workers/42061 and
> references.

I can't really find anything in that thread or its references that seems 
relevant.

Exporting readonly variables was already possible with POSIXBUILTINS 
off, so I don't see how it could harm anything to allow it with 
POSIXBUILTINS on.

Here is some more context than was provided by the patch. Note that all 
of this is only executed if POSIXBUILTINS is on.

     if (isset(POSIXBUILTINS)) {
         /*
          * Stricter rules about retaining readonly attribute in this case.
          */
         if ((on & (PM_READONLY|PM_EXPORTED)) &&
             (!usepm || (pm->node.flags & PM_UNSET)) &&
             !ASG_VALUEP(asg))
             on |= PM_UNSET;
         else if (usepm && (pm->node.flags & PM_READONLY) &&
                  !(on & PM_READONLY) && !(on & PM_EXPORTED)) {
             zerr("read-only variable: %s", pm->node.nam);
             return NULL;
         }
     }

As the comment says, that block applies stricter rules about retaining 
the readonly attribute for POSIXBUILTINS. I merely removed the 
restriction that readonly variables cannot be exported by adding:

	 && !(on & PM_EXPORTED)

Note that the restriction was already inconsistent: this code did not 
block unset readonly variables from being given the export flag.

FWIW, the test suite passes cleanly with this patch.

Thanks,

- M.


  reply	other threads:[~2018-04-21 12:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-18 19:58 Martijn Dekker
2018-04-18 20:07 ` Bart Schaefer
2018-04-21 12:57   ` Martijn Dekker [this message]
2019-06-20 18:47 ` [PATCH] posix_builtins: allow exporting a readonly Martijn Dekker
2019-06-21  8:59   ` Peter Stephenson
2019-06-22 11:54     ` Martijn Dekker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=88bb2098-0b83-a320-d4b9-44ef6143ea13@inlv.org \
    --to=martijn@inlv.org \
    --cc=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).