You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gb...@apache.org on 2013/10/06 13:03:45 UTC

svn commit: r1529609 - in /subversion/branches/invoke-diff-cmd-feature/subversion: libsvn_subr/io.c svn/svn.c tests/cmdline/diff_tests.py

Author: gbg
Date: Sun Oct  6 11:03:45 2013
New Revision: 1529609

URL: http://svn.apache.org/r1529609
Log:
On the invoke-diff-cmd branch: Ensure that delimiters that have a
  special start characer retain this after expansion.  Add new test.

* subversion/libsvn_subr/io.c

  (__create_custom_diff_cmd): Ensure that ineligible delimiters that
    have a special start characer are not modified.  Ensure that labels
    are passed correctly.

  (svn_io_run_diff2):  Add test location information.

* subversion/tests/cmdline/diff_tests.py

  (diff_invoke_external_diffcmd): Add new tests for the above behavior.

* subversion/svn/svn.c

  (svn_cl__options): Update help text pertaining to invoke-diff-cmd.



Modified:
    subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c
    subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c
    subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c?rev=1529609&r1=1529608&r2=1529609&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c Sun Oct  6 11:03:45 2013
@@ -3114,10 +3114,12 @@ __create_custom_diff_cmd(const char *lab
                          const char *cmd,
                          apr_pool_t *pool)
 {
-  /* The idea is that we can use the delimiter's characters to find
-     the correct indexes, and thus avoid repeated str(str|cmp|etc)
-     operation which easily turn this into an O(n^3) operation.  This
-     handrolled function is O(n).  */
+  /* 
+  We use the delimiter's characters to find the correct index.
+
+  This function can be tested by using the test 
+  /subversion/tests/cmdline/diff_tests.py diff_invoke_external_diffcmd
+  */
 
   struct replace_tokens_t
   {
@@ -3140,32 +3142,48 @@ __create_custom_diff_cmd(const char *lab
   words = svn_cstring_split(cmd, " ", TRUE, scratch_pool);
   
   result = apr_palloc(pool, 
-                      (words->nelts+1) * words->elt_size*sizeof(char *) );
+                      (words->nelts+1) * words->elt_size * sizeof(char *) );
 
   for (argv = 0; argv < words->nelts; argv++)
     {
       int j = 0;
       svn_stringbuf_t *word;
+      char special_start_marker = 0;  
+      char special_end_marker = 0;  
 
       word = svn_stringbuf_create_empty(scratch_pool);
       svn_stringbuf_appendcstr(word, APR_ARRAY_IDX(words, argv, char *));
       
-      /* a delimiter token always starts with a ';' */
-      if (word->data[j] == ';')  
-        {
+      /* If the first character of the string is a alphanumeric, it's
+         not a delimiter. */
+      if (!isalnum(word->data[j]))
+        {
+          /* check for the case that the first character is not a ';' and
+             that the second character is a ';' */
+          if ( (word->data[j] != ';') && (word->data[j+1] == ';') )
+            {
+              /* there are some cases where a '+' or other non-alphanumeric
+                 character can be prepended to a file name.  It's simpler if
+                 this is removed for the check and added back onto the result. */
+              special_start_marker = word->data[0]; 
+              svn_stringbuf_remove(word, 0, 1);   
+            }
+
+          /* Test that the first character is ';' and the string is a delimiter */
+          if (word->data[j] != ';')  /* this excludes switches etc, eg -u   */
+            goto end_of_isalnum_if;  /* this spares the code another indent */
+
           /* there are some cases where a '+' or other non-alphanumeric
              character can be appended to a file name.  It's simpler if
              this is removed for the check and added back onto the result. */
-          char special_end_marker = 0;  
-
-          /* if the last char is a non-alphanumeric, ie, + in ;f1+, remove it */
           if (!isalnum(word->data[word->len-1]))
             {
               special_end_marker = word->data[word->len-1]; 
               svn_stringbuf_remove(word, word->len-1, 1);   
             }
 
-          if (word->data[++j] == ';')  /* likely to be a protected delimiter */
+          /* A ';' in second place could be a protected delimiter */      
+          if (word->data[++j] == ';')  
             {
               /* ensure that there is an unbroken line of ';' */ 
               while (j < word->len && word->data[j++] == ';') ;
@@ -3181,7 +3199,6 @@ __create_custom_diff_cmd(const char *lab
                 {
                   /* it's a protected delimiter, so we consume one ';'  */
                   svn_stringbuf_remove(word, 0, 1);   
-                
                 }
             }
           else /* check that this is a regular delimiter (length 3) */
@@ -3206,12 +3223,17 @@ __create_custom_diff_cmd(const char *lab
                                       tokens_tab[j].token,
                                       strlen(tokens_tab[j].token));
               }
-
-          /* put the special character back if it exists */
-          if (special_end_marker) 
-            svn_stringbuf_appendbyte(word, special_end_marker);
-
         }
+    end_of_isalnum_if:
+
+      /* put the special start character back if it exists */
+      if (special_start_marker) 
+        svn_stringbuf_insert(word, 0, &special_start_marker, 1);
+
+      /* put the special end character back if it exists */
+      if (special_end_marker) 
+        svn_stringbuf_appendbyte(word, special_end_marker);
+      
       result[argv] = word->data;
     }  
   result[argv] = NULL;
@@ -3278,6 +3300,10 @@ svn_io_run_diff2(const char *dir,
                  const char *diff_cmd,
                  apr_pool_t *pool)
 {
+  /* 
+  This function can be tested by using the test 
+  /subversion/tests/cmdline/diff_tests.py diff_external_diffcmd
+  */
   svn_stringbuf_t *com;
   com = svn_stringbuf_create_empty(pool);
   svn_stringbuf_appendcstr(com, diff_cmd);

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c?rev=1529609&r1=1529608&r2=1529609&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c Sun Oct  6 11:03:45 2013
@@ -373,6 +373,12 @@ const apr_getopt_option_t svn_cl__option
                       "Any other instances of ';' or multiples thereof not    \n"
                       "                             "
                       "accompanied by f1, f2, l1 or l3 will not be modified.  \n"
+                      "                             "
+		      "Non-alphanumeric characters at the beginning or the end\n"
+                      "                             "
+		      "are permitted, ie. +;f1, ;f1+ and +;f1+                \n"
+
+
      )},
   {"internal-diff", opt_internal_diff, 0,
                        N_("override diff-cmd specified in config file")},

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py?rev=1529609&r1=1529608&r2=1529609&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py Sun Oct  6 11:03:45 2013
@@ -3281,26 +3281,39 @@ def diff_invoke_external_diffcmd(sbox):
   expected_output = svntest.verify.ExpectedOutput([
       "Index: iota\n",
       "===================================================================\n",
-      # assert correct label ;l1
+      # correct label ;l1 -> label 1
       "iota	(revision 1)\n",   
-      # assert correct file ;f1 
+
+      # correct file ;f1 -> file1
       os.path.abspath(svntest.wc.text_base_path("iota")) + "\n",
-      # assert correct label ;l2
+
+      # correct label ;l2 -> label 2
       "iota	(working copy)\n",
-      # assert correct file ;f2
+
+      # correct file ;f2 -> file2
       os.path.abspath("iota") + "\n",
-      # assert special end char is added ;f1+ -> file+
+
+      # special start char is retained +;f1 -> +file
+      "+" + os.path.abspath(svntest.wc.text_base_path("iota")) + "\n", 
+
+      # special end char is retained ;f1+ -> file+
       os.path.abspath(svntest.wc.text_base_path("iota")) + "+\n", 
-      ";f1\n",  # assert removal of ';' @ lower boundary '1'     ;;f1  -> ;f1
-      ";;l3\n", # assert removal of ';' @ high boundary '3'      ;;;l3 -> ;;l3
-      ";;fo\n", # assert only eligible delimiters are modified   ;;fo  -> ;;fo
-      ";f1o\n", # assert correct length test of delimiter        ;f1o  -> ;f1o
-      ";f15+\n" # assert non-modification of ineligble delimiter ;f15+ -> ;f15+
+
+      # special start and end char are retained +;f1+ -> +file+
+      "+"+os.path.abspath(svntest.wc.text_base_path("iota")) + "+\n", 
+
+      ";f1\n",  #  removal of ';' @ lower boundary '1'     ;;f1  -> ;f1
+      ";;l3\n", #  removal of ';' @ high  boundary '3'     ;;;l3 -> ;;l3
+      ";;fo\n", #  only eligible delimiters are modified   ;;fo  -> ;;fo
+      ";f1o\n", #  correct length test of delimiter        ;f1o  -> ;f1o
+      ";f15+\n",#  non-modification of ineligble delimiter ;f15+ -> ;f15+
+      "-u\n"    #  non-modification of ineligble element      -u -> -u
       ])
 
   svntest.actions.run_and_verify_svn(None, expected_output, [],
    'diff',
-   '--invoke-diff-cmd='+diff_script_path+' ;l1 ;f1 ;l2 ;f2 ;f1+ ;;f1 ;;;l3 ;;fo ;f1o ;f15+',
+   '--invoke-diff-cmd='+diff_script_path+
+      ' ;l1 ;f1 ;l2 ;f2 +;f1 ;f1+ +;f1+ ;;f1 ;;;l3 ;;fo ;f1o ;f15+ -u',
    iota_path)