[patch #6427] Syntax formatting utilities

[John Darrington]

Follow-up Comment #5, patch #6427 (project pspp):

I don't have any fundamental problems with this, but

1. Let's think of a better prefix.  "gen_string" is so vague, it could mean
almost anything.  How about syntax_fmt_* ?

2. I predict that gen_string will almost always be called with NULL as the
second arg, so why bother with it?  I can see that you wanted to have that arg
for consistency.  But I think it's somewhat inelegent and inefficient to have
most of the occurances passing in null, and then the function testing for null
and doing something different if it's not.  So let's split gen_string into 2
or 3 seperate functions, and make gen_string a wrapper around those 3.

3. In the comments for gen_string: "STR must be encoded ..." What is STR?

4. > int quote; This could be bool ?

5. In gen_pspp_valist, 
+        case 'd':
+          {
+            int i = va_arg (args, int);
+            ds_put_format (output, "%d", i);
+          }
+          break;

This will fail in the general case, eg: "%3d".  We can use gnulib's
printf-parse to help here.  On the other hand perhaps it's not worth doing
yet.

6. But at least let's make gen_pspp_valist accept "%%" in the conventional
fashion.

7.  In gen_{string, number, value etc} I suggest that we make the output
string the first parameter rather than the last.


8.  At the back of my mind, I have the idea that, in order to properly cope
with i18n issues, at some stage in the future, all syntax will have to
internally be converted to wchar_t *.  How would this patch affect that
decision if we took it?


    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?6427>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/