bugGNU Octave - Bugs: bug #41484, change error message from...

 
 

bug #41484: change error message from validatestring to follow Octave guidelines

Submitter:  Carnë Draug <carandraug>
Submitted:  Thu 06 Feb 2014 06:55:40 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 21 Mar 2014 01:59:17 PM UTC, comment #5: 

I modified your patch slightly and checked it in on the development branch of Octave (future 4.2 release).  You couldn't have known about it unless you spend a lot of time with Octave, but string concatenation is faster if you let the parser do it rather than calling out to a library function like sprintf.  So I changed that in two instances.  I also added your name to the list of contributors to Octave, which shows up in the preface of the Manual.  Thanks.

The final changeset is here (http://hg.savannah.gnu.org/hgweb/octave/rev/d227178a0d40).







Rik <rik5>
Group administrator
Fri 28 Feb 2014 06:27:00 PM UTC, comment #4: 

I've just added a second version of the patch. It doesn't mention validatestring any more if the error is not related to the script itself.

My first proposal assumed that in case additional arguments were missing it should have printed the previous message as a fallback, but now I see it is not the way it is meant to be.

Eugenio Gianniti <eg123>
Sat 22 Feb 2014 08:01:48 PM UTC, comment #3: 

Your patch adds error messages that make reference to validatestring.

For example, some of the error messages now start with "validatestring: '%s' does not match any of..." but the whole point of this function is to cause the error in this situation so the error message should not mention it.

I suggest a look at validateattributes (from Octave core) or validateLengthUnit (from the mapping package) for an example of what I mean.


Carnë Draug <carandraug>
Group Member
Thu 06 Feb 2014 08:30:31 PM UTC, comment #2: 

Good idea. I was gonna do it myself, but since it would only go into 4.2 anyway, let's see if anyone picks it up.

I added it to the short projects page


http://wiki.octave.org/Short_projects#Bugs

Carnë Draug <carandraug>
Group Member
Thu 06 Feb 2014 07:13:07 PM UTC, comment #1: 

This would be a goo one for a beginer to get his/her feet wet.

Doug Stewart <dastew>
Thu 06 Feb 2014 06:55:40 PM UTC, original submission:  

The function validatestring accepts an input string and a list of valid, and returns the one from the list that gives the best match, or throws an error if there's no good match.

I would like to change the error message so that it follows Octave's guideline better. This allows for Octave and Octave Forge functions to use it. Consider:


octave> b = validatestring ("foo", {"bar", "qux"}, "func", "VARNAME")
error: validatestring: Function: func Variable: VARNAME:
'foo' does not match any of
bar, qux
error: called from:
error:   /usr/local/share/octave/3.8.0/m/strings/validatestring.m at line 118, column 5


The start of the message suggests that it's an error in validatestring but what really happens is that validatestring() is throwing an error by instruction of "func". Also, to make it look more Octavish, I would like to replace it with:


error: func: VARNAME must match one the following:
bar, qux


Should I try to make this change?

Carnë Draug <carandraug>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #30736:  bug_41484_2.diff added by eg123 (3KiB - application/octet-stream)
file #30649:  bug-41484.diff added by mp57 (4KiB - application/octet-stream)
file #30640:  bug_41484.diff added by eg123 (3KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by eg123 (Updated the item)
  • -email is unavailable- added by mp57 (Updated the item)
  • -email is unavailable- added by dastew (Posted a comment)
  • -email is unavailable- added by carandraug (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-03-21 rik5 StatusNone Fixed
        Open/ClosedOpen Closed
    2014-02-28 eg123 Attached File- Added bug_41484_2.diff, #30736
    2014-02-23 mp57 Attached File- Added bug-41484.diff, #30649
    2014-02-22 eg123 Attached File- Added bug_41484.diff, #30640

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code