patchPSPP - Patches: patch #5511, Clean up variable formats

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #5511: Clean up variable formats

Submitter:  Ben Pfaff <blp>
Submitted:  Wed 01 Nov 2006 04:20:36 AM UTC
   
 
Category:  None Item Group:  None
Status:  Done Assigned to:  blp
Open/Closed:  Closed

Fri 03 Nov 2006 04:53:51 AM UTC, comment #2: 

Thank you for the comments.

> * Will the // comments in operations.def be OK with a C89 compiler?


These comments don't make it to the compiler.  They get dropped by the Perl code that translates operations.def into various C source files.

> * There are some msg strings in format.c which should probably use ngettext
> instead of gettext, to ensure that plural forms can be translated. Eg:
> "allows at most %d decimals."


Good spotting!  Fixed.

> * fmt_date_template: There is an analagous function in
> src/ui/gui/var-type-dialog.c called dollar_format_template. Would it make
> sense to put these in the same file ?


We could do something like that, sure.  Would it make sense to generalize it, to make a function that returns a "picture" of what a format outputs, for any given format?

> * In src/data/data-in.c, why do the FMT_CC? values assert NOT_REACHED() ? Is
> it not feasible to input a custom currency value ?


SPSS disallows using custom currency formats for input.  I think that's probably due to ambiguity issues.  I wasn't planning to change it; it doesn't seem too useful, and we'd have to define the semantics of what it means.

Ben Pfaff <blp>
Group administrator
Fri 03 Nov 2006 03:34:23 AM UTC, comment #1: 

I can't see any major issues.  Some minor ones:

  • Will the // comments in operations.def be OK with a C89 compiler?
  • There are some msg strings in format.c which should probably use ngettext instead of gettext, to ensure that plural forms can be translated. Eg: "allows at most %d decimals."
  • fmt_date_template: There is  an analagous function in src/ui/gui/var-type-dialog.c called dollar_format_template.  Would it make sense to put these in the same file ?
  • In src/data/data-in.c, why do the FMT_CC? values assert NOT_REACHED() ?  Is it not feasible to input a custom currency value ?


John Darrington <jmd>
Group administrator
Wed 01 Nov 2006 04:20:36 AM UTC, original submission:  

This change set cleans up src/data/format.[ch] and deals with the direct fallout.  It will be followed up by a change set that fixes data_out and adds a thorough collection of tests to prevent regressions, then by a change set that does similarly for data_in.

The patch is difficult to read in the case of src/data/format.[ch], so I've also attached clean copies of the revised files.

Comments?

Ben Pfaff <blp>
Group administrator

 

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

Attached Files
file #11125:  format.tar.gz added by blp (6KiB - application/gzip - clean copies of revised src/data/format.[ch])
file #11124:  format.patch added by blp (118KiB - text/x-patch - patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

CC list is empty

 

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.

 

Follow 5 latest changes.

Date Changed by Updated Field Previous Value => Replaced by
2006-11-03 blp StatusReady For Test/Review Done
    Open/ClosedOpen Closed
2006-11-03 jmd Assigned tojmd blp
2006-11-01 blp Attached File- Added format.tar.gz, #11125
2006-11-01 blp Attached File- Added format.patch, #11124

Back to the top

Powered by Savane 3.13-02a9.
Corresponding source code