You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Lundblad <pl...@google.com> on 2007/01/31 08:08:57 UTC

Re: svn commit: r23303 - in trunk/subversion: include libsvn_subr tests/libsvn_subr

cmpilato@tigris.org writes:
> Modified: trunk/subversion/include/svn_path.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_path.h?pathrev=23303&r1=23302&r2=23303
> ==============================================================================
> --- trunk/subversion/include/svn_path.h	(original)
> +++ trunk/subversion/include/svn_path.h	Tue Jan 30 14:16:14 2007
> @@ -116,6 +116,18 @@
>   */
>  char *svn_path_dirname(const char *path, apr_pool_t *pool);
>  
> +/** Split @c path into a root portion and an extension such that the
> + * the root + the extension = the original path, and where the

s/@c path/@a path/
Double "the".

> Modified: trunk/subversion/libsvn_subr/path.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/path.c?pathrev=23303&r1=23302&r2=23303
> ==============================================================================
> --- trunk/subversion/libsvn_subr/path.c	(original)
> +++ trunk/subversion/libsvn_subr/path.c	Tue Jan 30 14:16:14 2007
> @@ -1394,3 +1394,40 @@
>  
>    return SVN_NO_ERROR;
>  }
> +
> +void
> +svn_path_splitext(const char **path_root,
> +                  const char **path_ext,
> +                  const char *path,
> +                  apr_pool_t *pool)
> +{
> +  const char *last_period, *last_slash;
> +
> +  /* Do we even have a period in this thing?  And if so, is there
> +     anything after it?  */
> +  last_period = strrchr(path, '.');
> +  if (last_period && (last_period + 1 != '\0'))
> +    {
> +      /* If we have a period, we need to make sure it occurs in the
> +         final path component (otherwise, it doesn't count).  If so,
> +         we've found our separator. */
> +      last_slash = strrchr(path, '/');
> +      if ((! last_slash) || (last_period > last_slash))

Could simplify by starting looking for the slash at last_period.

> Modified: trunk/subversion/tests/libsvn_subr/path-test.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/libsvn_subr/path-test.c?pathrev=23303&r1=23302&r2=23303
> ==============================================================================
> --- trunk/subversion/tests/libsvn_subr/path-test.c	(original)
> +++ trunk/subversion/tests/libsvn_subr/path-test.c	Tue Jan 30 14:16:14 2007
> @@ -1364,6 +1365,58 @@
...
> +  for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++)
> +    {
> +      const char *path = tests[i].path;
> +      const char *path_root;
> +      const char *path_ext;
> +
> +      svn_pool_clear(subpool);
> +      svn_path_splitext(&path_root, &path_ext, path, subpool);
> +
> +      if ((strcmp(tests[i].path_root, path_root))
> +          || (strcmp(tests[i].path_ext, path_ext)))
> +        return svn_error_createf
> +          (SVN_ERR_TEST_FAILED, NULL,
> +           "svn_path_splitext (%s) returned ('%s', '%s') "
> +           "instead of ('%s', '%s')",
> +           tests[i].path, path_root, path_ext, 
> +           tests[i].path_root, tests[i].path_ext);
> +    }
> +  svn_pool_destroy(subpool);
> +  return SVN_NO_ERROR;

It would be good to test the combinations with NULL return arguments as
well.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r23303 - in trunk/subversion: include libsvn_subr tests/libsvn_subr

Posted by "C. Michael Pilato" <cm...@collab.net>.
Peter Lundblad wrote:
>> @@ -116,6 +116,18 @@
>>   */
>>  char *svn_path_dirname(const char *path, apr_pool_t *pool);
>>  
>> +/** Split @c path into a root portion and an extension such that the
>> + * the root + the extension = the original path, and where the
> 
> s/@c path/@a path/
> Double "the".

Got it.

>> +      last_slash = strrchr(path, '/');
>> +      if ((! last_slash) || (last_period > last_slash))
> 
> Could simplify by starting looking for the slash at last_period.

Of course!  Thanks.

> It would be good to test the combinations with NULL return arguments as
> well.

Good point.  Will do.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand