Mailing-List: contact stdcxx-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: stdcxx-dev@incubator.apache.org
Received-SPF: neutral (asf.osuosl.org: local policy)
Message-ID: <44D27401.2090802@roguewave.com>
Date: Thu, 03 Aug 2006 16:09:05 -0600
From: Martin Sebor <sebor@roguewave.com>
Organization: Rogue Wave Software
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20050920
MIME-Version: 1.0
To: stdcxx-dev@incubator.apache.org
Subject: Re: [PATCH] problems in eval_options()
References: <44D23732.8010808@roguewave.com> <44D266CC.2030803@roguewave.com>
In-Reply-To: <44D266CC.2030803@roguewave.com>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Andrew Black wrote:
> Greetings Martin
> 
> I've glanced over the patch and have attached a revised version that I 
> feel is a tad more consistent.

What? The consistency of my patch being challenged?! ;-)

> * Invalid/missing options error messages are now routed through the 
> (new) bad_value and missing_value functions.  I prefer these to calling 
> terminate(), as they also display the usage instructions when the option 
> is incorrectly used.

I don't think we should be displaying the entire help screen for
each typo. I have reservations about doing that for invalid option,
let alone for their arguments. The help text is long and might drown
out the error message itself. It's also not how most implementations
of utilities behave. So unless you have a really good argument for
doing so I'll remove this bit.

> * the -d and -x switches now check for a missing value for consistency 
> with the -t switch.

Okay.

> * use 'sizeof option - 1' rather than a magic constant for parameter 3 
> when calling get_long_val

Ah, I missed those. Good catch.

> * Removed superfluous assignment to optname for the compat and nocompat 
> switches (This one's debatable).

I tried to be (but wasn't) consistent about the assignment even
where it wasn't needed in case we made changes in the future
that assumed the variables were non-null. I suppose since we
don't set the variables for options like -q or -h we might as
well avoid setting them for these two.

> * Moved declaration of act struct in one conditional

Martin