You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2005/06/08 16:23:27 UTC
Re: svn commit: r15002 - in trunk/subversion: clients/cmdline include libsvn_client tests/clients/cmdline
julianfoad@tigris.org writes:
> --- trunk/subversion/include/svn_client.h (original)
> +++ trunk/subversion/include/svn_client.h Tue Jun 7 12:41:22 2005
> @@ -746,12 +763,31 @@
> * Use @a nonrecursive to indicate that imported directories should not
> * recurse into any subdirectories they may have.
> *
> + * If @a no_ignore is FALSE, don't add files or directories that match
> + * ignore patterns.
> + *
> * ### kff todo: This import is similar to cvs import, in that it does
> * not change the source tree into a working copy. However, this
> * behavior confuses most people, and I think eventually svn _should_
> * turn the tree into a working copy, or at least should offer the
> * option. However, doing so is a bit involved, and we don't need it
> - * right now.
> + * right now.
> + *
> + * @since New in 1.3.
> + */
> +svn_error_t *svn_client_import2 (svn_client_commit_info_t **commit_info,
> + const char *path,
> + const char *url,
> + svn_boolean_t nonrecursive,
> + svn_boolean_t no_ignore,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> +/**
> + * Similar to svn_client_import2(), but with the @a no_ignore parameter
> + * always set to @c FALSE.
> + *
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> */
> svn_error_t *svn_client_import (svn_client_commit_info_t **commit_info,
> const char *path,
Should we be saying "1.2 API" there, or "1.0 API"?
> --- trunk/subversion/libsvn_client/add.c (original)
> +++ trunk/subversion/libsvn_client/add.c Tue Jun 7 12:41:22 2005
> @@ -262,10 +262,23 @@
> return SVN_NO_ERROR;
> }
>
> +/* Schedule directory DIRNAME recursively for addition with access baton
> + * ADM_ACCESS.
> + *
> + * If DIRNAME (or any item below directory DIRNAME) is already scheduled for
> + * addition, add will fail and return an error unless FORCE is TRUE.
> + *
> + * Files and directories that match ignore patterns will not be added unless
> + * NO_IGNORE is TRUE.
> + *
> + * If CTX->CANCEL_FUNC is non-null, call it with CTX->CANCEL_BATON to allow
> + * the user to cancel the operation
> + */
> static svn_error_t *
> add_dir_recursive (const char *dirname,
> svn_wc_adm_access_t *adm_access,
> svn_boolean_t force,
> + svn_boolean_t no_ignore,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
Thank you for documenting this function!
You might want to state that the ignore patterns are obtained from CTX
(same in the public APIs). Not a big deal, just a non-expert reader
might wonder where the patterns come from.
> @@ -633,7 +641,7 @@
> SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
>
> SVN_ERR (svn_io_dir_make (path, APR_OS_DEFAULT, subpool));
> - err = svn_client_add (path, FALSE, ctx, subpool);
> + err = svn_client_add3 (path, FALSE, FALSE, FALSE, ctx, subpool);
>
> /* We just created a new directory, but couldn't add it to
> version control. Don't leave unversioned directoies behind. */
Why are ignores unconditional for svn_client_mkdir()? If ignore
patterns might be obeyed here (which the FALSE flag above implies),
then shouldn't we also offer an ability to override that?
Sorry if this was discussed already in the thread leading up to this
commit; I don't remember seeing anything about it.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15002 - in trunk/subversion: clients/cmdline include libsvn_client tests/clients/cmdline
Posted by Max Bowsher <ma...@ukf.net>.
kfogel@collab.net wrote:
> Julian Foad <ju...@btopenworld.com> writes:
>> kfogel@collab.net wrote:
>>> julianfoad@tigris.org writes:
>>>> + * @deprecated Provided for backward compatibility with the 1.2 API.
>>>> */
>>>> svn_error_t *svn_client_import (svn_client_commit_info_t **commit_info,
>>>> const char *path,
>>> Should we be saying "1.2 API" there, or "1.0 API"?
>>
>> I think I sometimes had a clear notion of which was more appropriate,
>> but right now I've no idea. Anyone?
>
> I guess I think earliest applicable interface is best.
I think the other way - you can deduce the earliest interface trivially -
it's either 1.0, or the value of any @since tag, if present. It's harder to
deduce the latest applicable interface, so it seems better to specify that.
I think most instances of @deprecated in our code currently document the
latest applicable interface.
Max.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15002 - in trunk/subversion: clients/cmdline include libsvn_client tests/clients/cmdline
Posted by kf...@collab.net.
Julian Foad <ju...@btopenworld.com> writes:
> kfogel@collab.net wrote:
> > Julian Foad <ju...@btopenworld.com> writes:
> >>I mentioned that that behaviour (only applicable in recursion) is not
> >>documented, and I'm not sure it's what I really want (but it's a hard
> >>call), but I don't suppose that comment is enough to make anyone
> >>document it ... :-)
> > I think I no longer have enough context to understand this paragraph,
> > but I'd be happy to volunteer to write a doc string if you can give me
> > a general pointer to what you had in mind (if that's what you meant).
>
> Thanks. What needs documenting is that the ignore lists are only
> applied to targets found by recursion, not to targets explicitly
> named, in all API functions that use them (such as add and import and
> status, at both client and wc levels). Sorry I don't have a list of
> exactly which functions.
Oh, noooooow I see why you were so happy to hand this off to me :-).
I don't think I have time to do that task either :-(.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15002 - in trunk/subversion: clients/cmdline include
libsvn_client tests/clients/cmdline
Posted by Julian Foad <ju...@btopenworld.com>.
kfogel@collab.net wrote:
> Julian Foad <ju...@btopenworld.com> writes:
>>I mentioned that that behaviour (only applicable in recursion) is not
>>documented, and I'm not sure it's what I really want (but it's a hard
>>call), but I don't suppose that comment is enough to make anyone
>>document it ... :-)
>
> I think I no longer have enough context to understand this paragraph,
> but I'd be happy to volunteer to write a doc string if you can give me
> a general pointer to what you had in mind (if that's what you meant).
Thanks. What needs documenting is that the ignore lists are only applied to
targets found by recursion, not to targets explicitly named, in all API
functions that use them (such as add and import and status, at both client and
wc levels). Sorry I don't have a list of exactly which functions.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15002 - in trunk/subversion: clients/cmdline include libsvn_client tests/clients/cmdline
Posted by kf...@collab.net.
Julian Foad <ju...@btopenworld.com> writes:
> kfogel@collab.net wrote:
> > julianfoad@tigris.org writes:
> >>+ * @deprecated Provided for backward compatibility with the 1.2 API.
> >> */
> >> svn_error_t *svn_client_import (svn_client_commit_info_t **commit_info,
> >> const char *path,
> > Should we be saying "1.2 API" there, or "1.0 API"?
>
> I think I sometimes had a clear notion of which was more appropriate,
> but right now I've no idea. Anyone?
I guess I think earliest applicable interface is best.
> That puzzled me when I reviewed the patch, but I investigated and
> found that ignores are only applied to items found by recursion, not
> to explicitly given targets. A directory made by "mkdir" is always an
> explicit target, so ignores don't apply.
Ah, thanks, that makes sense.
> I mentioned that that behaviour (only applicable in recursion) is not
> documented, and I'm not sure it's what I really want (but it's a hard
> call), but I don't suppose that comment is enough to make anyone
> document it ... :-)
> Any volunteers?
I think I no longer have enough context to understand this paragraph,
but I'd be happy to volunteer to write a doc string if you can give me
a general pointer to what you had in mind (if that's what you meant).
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15002 - in trunk/subversion: clients/cmdline include
libsvn_client tests/clients/cmdline
Posted by Julian Foad <ju...@btopenworld.com>.
kfogel@collab.net wrote:
> julianfoad@tigris.org writes:
>>+ * @deprecated Provided for backward compatibility with the 1.2 API.
>> */
>> svn_error_t *svn_client_import (svn_client_commit_info_t **commit_info,
>> const char *path,
>
> Should we be saying "1.2 API" there, or "1.0 API"?
I think I sometimes had a clear notion of which was more appropriate, but right
now I've no idea. Anyone?
>>+/* Schedule directory DIRNAME recursively for addition with access baton
>>+ * ADM_ACCESS.
>>+ *
>>+ * If DIRNAME (or any item below directory DIRNAME) is already scheduled for
>>+ * addition, add will fail and return an error unless FORCE is TRUE.
>>+ *
>>+ * Files and directories that match ignore patterns will not be added unless
>>+ * NO_IGNORE is TRUE.
>>+ *
>>+ * If CTX->CANCEL_FUNC is non-null, call it with CTX->CANCEL_BATON to allow
>>+ * the user to cancel the operation
>>+ */
>> static svn_error_t *
>> add_dir_recursive (const char *dirname,
>> svn_wc_adm_access_t *adm_access,
>> svn_boolean_t force,
>>+ svn_boolean_t no_ignore,
>> svn_client_ctx_t *ctx,
>> apr_pool_t *pool)
>> {
>
> Thank you for documenting this function!
>
> You might want to state that the ignore patterns are obtained from CTX
> (same in the public APIs). Not a big deal, just a non-expert reader
> might wonder where the patterns come from.
OK, I'll mark this message for attention and consider doing so.
>>@@ -633,7 +641,7 @@
>> SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
>>
>> SVN_ERR (svn_io_dir_make (path, APR_OS_DEFAULT, subpool));
>>- err = svn_client_add (path, FALSE, ctx, subpool);
>>+ err = svn_client_add3 (path, FALSE, FALSE, FALSE, ctx, subpool);
>>
>> /* We just created a new directory, but couldn't add it to
>> version control. Don't leave unversioned directoies behind. */
>
> Why are ignores unconditional for svn_client_mkdir()? If ignore
> patterns might be obeyed here (which the FALSE flag above implies),
> then shouldn't we also offer an ability to override that?
That puzzled me when I reviewed the patch, but I investigated and found that
ignores are only applied to items found by recursion, not to explicitly given
targets. A directory made by "mkdir" is always an explicit target, so ignores
don't apply.
I mentioned that that behaviour (only applicable in recursion) is not
documented, and I'm not sure it's what I really want (but it's a hard call),
but I don't suppose that comment is enough to make anyone document it ... :-)
Any volunteers?
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org