Merge lp:~zorba-coders/zorba/1025564 into lp:zorba

[Chris Hillery]

Added changes for deprecating -f option.

[Matthias Brantner (matthias-brantner)]

Works great. Some minor issues:

- the --help documentation should mention that the argument is deprecated
- also, the --help documentation should mention the syntax to pass the file
- please respect coding guidelines. For example,
if(qBeforeF()){ => if (qBeforeF())
{
- remove commented out code

review: Needs Fixing

[Chris Hillery (ceejatec)]

Agree about the --help doc; if I get a little time later I might put in a suggested change myself.

Also agree about not leaving commented-out code.

To be fair to Luis, the coding guidelines do not mention anything about brace positioning or spaces after "if", etc.

[Chris Hillery (ceejatec)]

Also, add a mention to the option changes to the ChangeLog.

review: Needs Fixing

[Luis Rodriguez Gonzalez (kuraru)]

Comments fixed, please re-review.

[Chris Hillery (ceejatec)]

I'm not sure why Launchpad is showing hundreds of changes on this branch; something odd when merging from the trunk, I assume. But the diff looks good, so if it merges OK, I'm happy.

review: Approve

[Sorin Marian Nasoi (sorin.marian.nasoi)]

> I'm not sure why Launchpad is showing hundreds of changes on this branch;
> something odd when merging from the trunk, I assume. But the diff looks good,
> so if it merges OK, I'm happy.

Since most of the changes are in bin folder, that has not been changed in a while now, just to be 100% sure that everything is OK one could:
- create a new branch of lp:zorba locally
- merge the proposed changes in this branch and propose it for merging (be sure to give it a unique name when pushing the changes to LP)
- delete this merge proposal and related branch

[Matthias Brantner (matthias-brantner)]

What happened to "Fixed bug #866958 (Parsing error not explicit enough)" in the ChangeLog?

review: Needs Information

[Chris Hillery (ceejatec)]

That bug was never part of this proposal. I think it's showing up in the did here due to my merge from the trunk. The Changelog had a number of things get rearranged which made conflict resolution hard. That bug is in fact fixed so I assume it should be in the Changelog.

[Matthias Brantner (matthias-brantner)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/1025564-2013-04-30T22-40-03.37Z/log.html

[Zorba Build Bot (zorba-buildbot)]

The attempt to merge lp:~zorba-coders/zorba/1025564 into lp:zorba failed. Below is the output from the failed tests.

CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275 (message):
  Validation queue job 1025564-2013-04-30T22-40-03.37Z is finished. The
  final status was:

  1 tests did not succeed - changes not commited.

Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

[Zorba Build Bot (zorba-buildbot)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/1025564-2013-05-01T03-32-07.037Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job 1025564-2013-05-01T03-32-07.037Z is finished. The final status was:

All tests succeeded!

=== modified file 'ChangeLog'
--- ChangeLog	2013-04-30 23:54:17 +0000
+++ ChangeLog	2013-05-01 03:16:28 +0000
@@ -94,6 +94,7 @@
   * Fixed bug in computing the static type of a self-axis path step.
   * Fixed bug #867068: (Incorrect usage of XQDY0027)
   * Fixed bug: jsoniq constructor should not be const-folded
+  * Fixed bug #866958 (Parsing error not explicit enough)
   * Fixed bug #1152834 (bug in computing the scripting kind of array and object
     constructor exprs)
   * Fixed bug #1125444 (input group-by exprs were not treated by index-flwor_vars()
@@ -101,6 +102,7 @@
   * Raise XQST0046, if needed, from namespace declaration attribute inside
     direct element constructor.
   * Fixed bug #1023362 (xsi:type attribute ignored during validation)
+  * Fixed bug #1025564 (Deprecate -f argument to zorbacmd)
   * Fixed bug #1082740 (support for INF and -INF values in fn:subsequence function).  
 
 
 
=== modified file 'bin/CMakeLists.txt'
--- bin/CMakeLists.txt	2013-02-07 17:24:36 +0000
+++ bin/CMakeLists.txt	2013-05-01 03:16:28 +0000
@@ -58,6 +58,12 @@
 ENDIF(UNIX)
 
 # test the basic features of the command line tool
+ZORBA_ADD_TEST(bin/zorba_f1 zorbacmd blub.xq)
+ZORBA_SET_TEST_PROPERTY(bin/zorba_f1 PASS_REGULAR_EXPRESSION ".*file {blub.xq} not found or not readable.*")
+
+ZORBA_ADD_TEST(bin/zorba_f2 zorbacmd "${CMAKE_CURRENT_SOURCE_DIR}/test/option.xq" --option "{http://www.zorba-xquery.com}option=highly_unique")
+ZORBA_SET_TEST_PROPERTY(bin/zorba_f2 PASS_REGULAR_EXPRESSION "highly_unique")
+
 ZORBA_ADD_TEST(bin/zorba1 zorbacmd --omit-xml-declaration -q "1+1")
 ZORBA_SET_TEST_PROPERTY(bin/zorba1 PASS_REGULAR_EXPRESSION "2")
 
@@ -71,7 +77,14 @@
 ZORBA_SET_TEST_PROPERTY(bin/zorba4 PASS_REGULAR_EXPRESSION "(.*<child>child1</child>)*")
 
 ZORBA_ADD_TEST(bin/zorba5 zorbacmd -q -f blub.xq)
-ZORBA_SET_TEST_PROPERTY(bin/zorba5 PASS_REGULAR_EXPRESSION ".*Extra arguments found on command line.*")
+ZORBA_SET_TEST_PROPERTY(bin/zorba5 PASS_REGULAR_EXPRESSION ".*No queries submitted.*")
+
+IF (UNIX)
+  CONFIGURE_FILE ("${CMAKE_CURRENT_SOURCE_DIR}/test/shebang.xq.in"
+    "${CMAKE_CURRENT_BINARY_DIR}/shebang.xq" @ONLY)
+  ADD_TEST (bin/shebang "${CMAKE_CURRENT_BINARY_DIR}/shebang.xq")
+  SET_TESTS_PROPERTIES (bin/shebang PROPERTIES PASS_REGULAR_EXPRESSION "shebang success")
+ENDIF (UNIX)
 
 # test the compile-only option of the command line tool which is used for semantic checking in eclipse
 ZORBA_ADD_TEST(bin/zorba_compilechk1 zorbacmd -q "${CMAKE_CURRENT_SOURCE_DIR}/test/mymod.xq" -f -l)
 
=== added file 'bin/test/shebang.xq.in'
--- bin/test/shebang.xq.in	1970-01-01 00:00:00 +0000
+++ bin/test/shebang.xq.in	2013-05-01 03:16:28 +0000
@@ -0,0 +1,2 @@
+#!@ZORBA_EXE@
+"shebang success"
 
=== modified file 'bin/zorbacmdproperties.cpp'
--- bin/zorbacmdproperties.cpp	2013-02-07 17:24:36 +0000
+++ bin/zorbacmdproperties.cpp	2013-05-01 03:16:28 +0000
@@ -23,22 +23,21 @@
 
 std::string ZorbaCMDProperties::check_args () {
 
-  if(queriesOrFilesBegin() == queriesOrFilesEnd()) {
-    return "No queries submitted \nUsage: zorba -q '1 + 1' execute an inline query \n zorba -f -q file.xq execute a query from a file \n Use -h for help.";
+  if(queriesOrFilesBegin() == queriesOrFilesEnd())
+  {
+    return "No queries submitted \nUsage: zorba -q '1 + 1' execute an inline query \n zorba file.xq execute a query from a file \n Use -h for help.";
   }
 
-  if (getPositionalArgs ().size () != 0) {
-    QueriesOrFiles_t::const_iterator lIter;
-    for (lIter = queriesOrFilesBegin(); lIter != queriesOrFilesEnd();
-         ++lIter)
-    {
-      if (*lIter == "-f")
-        return "Extra arguments found on command line. Possible reason: misplaced -f option (try '-f -q filename' instead of '-q -f filename'). Use -h for help.";
-    }
-    
+  if(unknownOption())
+  {
     return "Extra arguments found on command line. Use -h for help.";
   }
 
+  if(queriesOrFilesBegin() == queriesOrFilesEnd())
+  {
+    return "No queries submitted \nUsage: zorba -q '1 + 1' execute an inline query \n zorba file.xq execute a query from a file \n Use -h for help.";
+  }
+
   if ( theBoundarySpace.size() != 0 )
   {
     if ( ! (theBoundarySpace.compare("strip") == 0 || theBoundarySpace.compare("preserve") == 0 ))
 
=== modified file 'bin/zorbacmdproperties_base.h'
--- bin/zorbacmdproperties_base.h	2013-02-07 17:24:36 +0000
+++ bin/zorbacmdproperties_base.h	2013-05-01 03:16:28 +0000
@@ -94,6 +94,9 @@
   bool theSerializePlan;
   bool theSavePlan;
   bool theLoadPlan;
+  bool theFparm;
+  bool theQBeforeF;
+  bool theUnknownOption;
 
   void initialize () 
   {
@@ -106,7 +109,7 @@
     theByteOrderMark = false;
     theOmitXmlDeclaration = false;
     theMultiple = 1;
-    theAsFiles = false;
+    theAsFiles = true;
     theOptimizationLevel = "O1";
     theLibModule = false;
     theParseOnly = false;
@@ -121,6 +124,9 @@
     theSerializePlan = false;
     theSavePlan = false;
     theLoadPlan = false;
+    theFparm = false;
+    theQBeforeF = false;
+    theUnknownOption = false;
   }
 
 public:
@@ -167,6 +173,8 @@
   const bool& serializePlan () const { return theSerializePlan; }
   const bool& loadPlan () const { return theLoadPlan; }
   const bool& savePlan () const { return theSavePlan; }
+  const bool& qBeforeF () const { return theQBeforeF; }
+  const bool& unknownOption() const { return theUnknownOption; }
 
   std::string load_argv (int argc, const char **argv) 
   {
@@ -279,6 +287,13 @@
       else if (strcmp (*argv, "--query") == 0 || strncmp (*argv, "-q", 2) == 0) 
       {
         int d = 2;
+        if(theFparm == false)
+          theAsFiles = false;
+        if(!strncmp(*(argv+1), "-f", 2))
+        {
+          theQBeforeF = true; // is it "-q -f <filename>" perhaps?
+          break;           // stop functionality here
+        }
         if ((*argv) [1] == '-' || (*argv) [2] == '\0') { d = 0; ++argv; }
         if (*argv == NULL)
         {
@@ -288,6 +303,7 @@
       }
       else if (strcmp (*argv, "--as-files") == 0 || strncmp (*argv, "-f", 2) == 0) 
       {
+        theFparm = true;
         theAsFiles = true;
       }
       else if (strcmp (*argv, "--external-variable") == 0 || strncmp (*argv, "-e", 2) == 0) 
@@ -449,12 +465,13 @@
       }
       else if ((*argv) [0] == '-')
       {
-        result = "unknown command line option "; result += *argv; break; 
+        result = "unknown command line option "; result += *argv; 
+        theUnknownOption = true; break; 
       }
       else
       {
+        init_val(*argv, theQueriesOrFiles, 0);
         copy_args (argv);
-        break;
       }
     }
 
@@ -481,7 +498,7 @@
         "--ordering-mode\nSet the ordering mode ('ordered' or 'unordered') in the static context.\n\n"
         "--multiple, -m\nExecute the given queries multiple times.\n\n"
         "--query, -q\nQuery test or file URI (file://...)\n\n"
-        "--as-files, -f\nTreat all -q arguments as file paths instead of URIs or inline queries.\n\n"
+        "--as-files, -f\nTreat all -q arguments as file paths instead of URIs or inline queries. This option is deprecated and will be defaulted to true in the future, so any entry in the command line is going to be treated as files.\n\n"
         "--external-variable, -e\nProvide the value for a variable given a file (name=file) or a value (name:=value)\n\n"
         "--context-item\nSet the context item to the XML document in a given file.\n\n"
         "--optimization-level\nOptimization level for the query compiler (O0, O1 or O2 - default: O1)\n\n"
@@ -504,7 +521,7 @@
         "--thesaurus\nMapping specifying a thesaurus URI to another.\n\n"
         "--serialize-plan, -s\nSerialize and then load the query execution plan.\n\n"
         "--compile-plan,\nDo not execute the query; just compile it and save the execution plan in the file specified with the -o option.\n\n"
-        "--execute-plan\nDo not compile the query; instead load the execution plan from the file specified by the -f -q options, and execute the loaded plan.\n\n"
+        "--execute-plan\nDo not compile the query; instead load the execution plan from the file specified by the -f -q options (or by any file specified without any other argument), and execute the loaded plan.\n\n"
         "--disable-http-resolution\nDo not use HTTP to resolve URIs\n\n"
         ;
   }