[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

PSPP-BUG: [bug #18705] Implement INSERT command.

From: Ben Pfaff
Subject: PSPP-BUG: [bug #18705] Implement INSERT command.
Date: Wed, 05 Sep 2007 04:39:43 +0000
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20061205 Iceweasel/ (Debian-

Update of bug #18705 (project pspp):

                  Status:   Ready for Test/Review => Works For Me           


Follow-up Comment #3:

Thanks for making this work!  It looks good.  I have a bunch of mostly
editorial comments.

In the manual:

Could BATCH mode and INTERACTIVE mode simply be called batch and
interactive mode?  I don't think that the capitals add any

This line:

+Certain commands, such as @cmd{INSERT}, (@ref{INSERT}) may explicitly

I would write as:

+Certain commands, such as @cmd{INSERT} (@pxref{INSERT}), may explicitly

(Note the position of the comma and use of @pxref.)

It looks like we should remove the documentation that we support "@"
as a way to include syntax files.  I think this is an SPSS/PC+-only
feature that SPSS does not have (early versions of PSPP targeted
SPSS/PC+ instead of SPSS), and thus I removed it from PSPP.  It seems
that I missed the mention of it in the manual.

Could you add a cross-reference to INSERT from the INCLUDE node?

In the INSERT code, could you change (@ref{INCLUDE}) to

In do_parse_command, I think that the change as written does not
completely make sense.  The "goto"s that are inserted will only change
the effect of the function if they change "result" to CMD_FAILURE, but
none of them change it to CMD_FAILURE.  Is that really what was
intended?  Perhaps the "result = CMD_FINISH;" statements should be
"result = CMD_FAILURE;"?  Or perhaps "if (result == CMD_FAILURE)"
should be "if (cmd_result_is_failure (result))"?

In cmd_insert, I would call lex_error in place of at least the final
call to msg, possibly other calls too although there are i18n issues
that need to be worked out for lex_error in those cases.  

I'm happy with the rest.


Reply to this item at:


  Message sent via/by Savannah

reply via email to

[Prev in Thread] Current Thread [Next in Thread]