Re: [PATCH] issue 2544 : Add support for 'blame' of WORKING revision

Hyrum K. Wright wrote:
> Thomas Leclaire wrote:
>> Hello,
>>
>> Here's a patch for the issue 2544 : "Add support for 'blame' of WORKING
>> revision", which was "svn_client_blame3() and svn_opt_revision_working".
>
> Has anybody had a chance to review and comment on Thomas's patch? If nothing
> happens in the next few days, I'll go ahead and attach it to Issue #2544.

Thomas,
Thanks for the patch. I've gone ahead and attached it to issue 2544 so
it doesn't get forgotten.

-Hyrum

>> [[[
>> Fix issue #2544: Add support for 'blame' of WORKING revision
>>
>> Add -r WORKING option in command line.
>> svn blame -rWORKING look at local modifications for the blame.
>>
>> * subversion/libsvn_subr/opt.c
>> (revision_from_word): Add option -r WORKING in command line
>>
>> * subversion/include/svn_client.h
>> (svn_client_blame3): Remove doc string which tells that
>> op_revision_working give error.
>>
>> * subversion/libsvn_client/blame.c
>> (svn_client_blame3): Remove SVN_ERR_UNSUPPORTED_FEATURE for attempts
>> to call 'blame' on the WORKING revision. Add support for this function.
>>
>> * subversion/libsvn_client/cat.c
>> (svn_client_cat2): Add option svn_opt_revision_working. It's not for
>> this issue but i've not found an other issue and it's a real minor
>> update discovered during tests.
>>
>>
>> Found by: Stefan Küng <tortoisesvn@gmail.com>
>> Suggested by: julianfoad, cmpilato, peter n lundblad
>>
>> ]]]
>>
>>
>> Index: subversion/libsvn_subr/opt.c
>> ===================================================================
>> --- subversion/libsvn_subr/opt.c (revision 23078)
>> +++ subversion/libsvn_subr/opt.c (working copy)
>> @@ -493,6 +493,8 @@
>> *
>> * - For "committed", set REVISION->kind to svn_opt_revision_committed.
>> *
>> + * - For "working", set REVISION->kind to svn_opt_revision_working.
>> + *
>> * If match, return 0, else return -1 and don't touch REVISION.
>> */
>> static int
>> @@ -514,6 +516,10 @@
>> {
>> revision->kind = svn_opt_revision_committed;
>> }
>> + else if (strcasecmp(word, "working") == 0)
>> + {
>> + revision->kind = svn_opt_revision_working;
>> + }
>> else
>> return -1;
>>
>>
>> Index: subversion/include/svn_client.h
>> ===================================================================
>> --- subversion/include/svn_client.h (revision 23078)
>> +++ subversion/include/svn_client.h (working copy)
>> @@ -1292,11 +1292,9 @@
>> * WC targets.
>> *
>> * If @a start->kind or @a end->kind is @c svn_opt_revision_unspecified,
>> - * return the error @c SVN_ERR_CLIENT_BAD_REVISION. If either are @c
>> - * svn_opt_revision_working, return the error @c
>> - * SVN_ERR_UNSUPPORTED_FEATURE. If any of the revisions of @a
>> - * path_or_url have a binary mime-type, return the error @c
>> - * SVN_ERR_CLIENT_IS_BINARY_FILE, unless @a ignore_mime_type is TRUE,
>> + * return the error @c SVN_ERR_CLIENT_BAD_REVISION. If any of the
>> + * revisions of @a path_or_url have a binary mime-type, return the error
>> + * @c SVN_ERR_CLIENT_IS_BINARY_FILE, unless @a ignore_mime_type is TRUE,
>> * in which case blame information will be generated regardless of the
>> * MIME types of the revisions.
>> *
>>
>>
>> Index: subversion/libsvn_client/blame.c
>> ===================================================================
>> --- subversion/libsvn_client/blame.c (revision 23078)
>> +++ subversion/libsvn_client/blame.c (working copy)
>> @@ -515,11 +515,6 @@
>> || end->kind == svn_opt_revision_unspecified)
>> return svn_error_create
>> (SVN_ERR_CLIENT_BAD_REVISION, NULL, NULL);
>> - else if (start->kind == svn_opt_revision_working
>> - || end->kind == svn_opt_revision_working)
>> - return svn_error_create
>> - (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
>> - _("blame of the WORKING revision is not supported"));
>> /* Get an RA plugin for this filesystem object. */
>> SVN_ERR(svn_client__ra_session_from_path(&ra_session, &end_revnum,
>> @@ -573,6 +568,38 @@
>> SVN_ERR(err);
>> + /* Add the blame corresponding to the locale working copy
>> + for the option -r WORKING */
>> + if (start->kind == svn_opt_revision_working
>> + || end->kind == svn_opt_revision_working)
>> + {
>> + svn_wc_adm_access_t *adm_access;
>> + svn_wc_status2_t *status;
>> +
>> + /* test if the locale file is modified */
>> + SVN_ERR(svn_wc_adm_open3(&adm_access, NULL,
>> + svn_path_dirname(target, pool),
>> + FALSE, 0, ctx->cancel_func,
>> + ctx->cancel_baton, pool));
>> + SVN_ERR(svn_wc_status2(&status, target, adm_access, pool));
>> + if (status->text_status != svn_wc_status_normal)
>> + {
>> + frb.rev = apr_palloc(pool, sizeof(*(frb.rev)));
>> + /* Maybe it could be possible to have 1234M as in local cat
>> + * However blame show revision number and next user
>> + * (here : "(local)"), so we see immediately that it is modified
>> + * but maybe a problem for svn clients (tortoise...) ?? */
>> + frb.rev->revision = frb.end_rev;
>> + frb.rev->author = apr_pstrdup(pool, "(local)");
>> + frb.rev->date = apr_pstrdup(pool, "today"); /* useful ?? */
>> + frb.rev->next = NULL;
>> + frb.rev->path = target;
>> +
>> + SVN_ERR(add_file_blame(frb.last_filename, target, &frb));
>> + frb.last_filename = target;
>> + }
>> + }
>> +
>> /* Report the blame to the caller. */
>> /* The callback has to have been called at least once. */
>>
>>
>> Index: subversion/libsvn_client/cat.c
>> ===================================================================
>> --- subversion/libsvn_client/cat.c (revision 23078)
>> +++ subversion/libsvn_client/cat.c (working copy)
>> @@ -177,9 +178,11 @@
>> if (! svn_path_is_url(path_or_url)
>> && (peg_revision->kind == svn_opt_revision_base
>> || peg_revision->kind == svn_opt_revision_committed
>> + || peg_revision->kind == svn_opt_revision_working
>> || peg_revision->kind == svn_opt_revision_unspecified)
>> && (revision->kind == svn_opt_revision_base
>> || revision->kind == svn_opt_revision_committed
>> + || revision->kind == svn_opt_revision_working
>> || revision->kind == svn_opt_revision_unspecified))
>> {
>> svn_wc_adm_access_t *adm_access;
>>
>>
>>
>> I tested this on command line with different configurations of svn
>> repository and it seems work. However i'm not an usual user of svn and i
>> don't know all ways to use blame. I realized this patch for a project
>> during my studies.
>>
>> Especially, I put the revision number for local version equal to last
>> revision updated. The M added as in cat function is more complex to
>> implements here, also in blame function we have rev_number and next
>> (local) so the M isn't necessary. So it's not a problem for command line
>> -rworking but maybe it causes trouble for some others uses.
>>
>> Also, the option -r WORKING is okay for blame and cat. diff give an
>> adapted error but for others i don't know.
>>
>> Thanks and sorry for my english! ;)
>>
>> --
>> Thomas Leclaire