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

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.

-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
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>