You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Kueng <to...@gmail.com> on 2017/02/04 08:23:06 UTC

wish for new API or extended one

Hi,

while trying out the new conflict resolver APIs I came upon a slight 
problem. The API svn_client_conflict_option_get_description() returns a 
string that can be used to show to the user as a choice - in TSVN that 
would be the text on a two-line button (the 'label' is the first line, 
the 'description' the second line).

The problem I'm having now is that the description is always the same, 
even if there are multiple 'moved-to-candidates' available. And if we 
use another than the first moved-to-candidate, the description mentions 
the wrong path.

Would it be possible to extend the API 
svn_client_conflict_option_get_description() so that it takes another 
parameter that's the index of the moved-to-candidate? Something like this:

svn_client_conflict_option_get_description(
         svn_client_conflict_option_t *option,
         int moved_to_candidate_index,
         apr_pool_t *result_pool);
with the moved_to_candidate_index set to -1 if not used.

Right now I'm working around this by parsing the description string for 
the second path and replacing it with the moved_to_candidate path that 
is used. But that's an ugly hack, and I'm not sure it will survive all 
translations of that string.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Kueng <to...@gmail.com>.

On 08.02.2017 15:03, Evgeny Kotkov wrote:
> Stefan Sperling <st...@elego.de> writes:
>
>> Ah, indeed. Thank you very much for this patch!
>> Please commit it when you get a chance.
>
> Thanks, I committed the fix in r1782169.
>
> Stefan Kueng <to...@gmail.com> writes:
>
>> But as I mentioned: without getting the options again with
>> svn_client_conflict_tree_get_resolution_options, the strings are not
>> updated.
>
> Stefan, could you please update to the HEAD revision and confirm that it
> fixes the issue you were seeing?

Yes, that solves my problems. Thanks!

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Stefan Sperling <st...@elego.de> writes:

> Ah, indeed. Thank you very much for this patch!
> Please commit it when you get a chance.

Thanks, I committed the fix in r1782169.

Stefan Kueng <to...@gmail.com> writes:

> But as I mentioned: without getting the options again with
> svn_client_conflict_tree_get_resolution_options, the strings are not
> updated.

Stefan, could you please update to the HEAD revision and confirm that it
fixes the issue you were seeing?


Thanks,
Evgeny Kotkov

Re: wish for new API or extended one

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Feb 08, 2017 at 12:30:03AM +0300, Evgeny Kotkov wrote:
> I think that the actual problem here is that calling the
> svn_client_conflict_option_set_moved_to_repos_relpath() function
> does not update the description (and the target path in the description
> could have changed).
> 
> In other words, probably we're looking at something along these lines:

Ah, indeed. Thank you very much for this patch!
Please commit it when you get a chance.

Thanks,
Stefan

> [[[
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 1782081)
> +++ subversion/include/svn_client.h (working copy)
> @@ -4495,6 +4495,7 @@ svn_error_t *
>  svn_client_conflict_option_set_moved_to_repos_relpath(
>    svn_client_conflict_option_t *option,
>    int preferred_move_target_idx,
> +  svn_client_ctx_t *ctx,
>    apr_pool_t *scratch_pool);
> 
>  /**
> Index: subversion/libsvn_client/conflicts.c
> ===================================================================
> --- subversion/libsvn_client/conflicts.c (revision 1782081)
> +++ subversion/libsvn_client/conflicts.c (working copy)
> @@ -9414,6 +9414,7 @@ svn_error_t *
>  svn_client_conflict_option_set_moved_to_repos_relpath(
>    svn_client_conflict_option_t *option,
>    int preferred_move_target_idx,
> +  svn_client_ctx_t *ctx,
>    apr_pool_t *scratch_pool)
>  {
>    svn_client_conflict_t *conflict = option->conflict;
> @@ -9466,6 +9467,14 @@ svn_client_conflict_option_set_moved_to_repos_relp
>        if (strcmp(move_target_repos_relpath, repos_relpath) == 0)
>          {
>            details->move_target_repos_relpath = repos_relpath;
> +          /* Update option description. */
> +          SVN_ERR(describe_incoming_move_merge_conflict_option(
> +                    &option->description,
> +                    conflict, ctx,
> +                    details,
> +                    conflict->pool,
> +                    scratch_pool));
> +
>            return SVN_NO_ERROR;
>          }
>      }
> Index: subversion/svn/conflict-callbacks.c
> ===================================================================
> --- subversion/svn/conflict-callbacks.c (revision 1782081)
> +++ subversion/svn/conflict-callbacks.c (working copy)
> @@ -1751,16 +1751,9 @@ handle_tree_conflict(svn_boolean_t *resolved,
>            if (conflict_option)
>              {
>                SVN_ERR(svn_client_conflict_option_set_moved_to_repos_relpath(
> -                        conflict_option, preferred_move_target_idx, iterpool));
> +                        conflict_option, preferred_move_target_idx,
> +                        ctx, iterpool));
> 
> -              /* Update option description. */
> -              SVN_ERR(build_tree_conflict_options(
> -                        &tree_conflict_options,
> -                        &possible_moved_to_repos_relpaths,
> -                        &possible_moved_to_abspaths,
> -                        NULL, conflict, ctx,
> -                        scratch_pool, scratch_pool));
> -
>                /* Update conflict description. */
>                SVN_ERR(svn_client_conflict_tree_get_description(
>                         &incoming_change_description, &local_change_description,
> ]]]
> 
> 
> Regards,
> Evgeny Kotkov

Re: wish for new API or extended one

Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Stefan Sperling <st...@elego.de> writes:

> I'm guessing that you are mixing up repository paths and working copy paths.
> Perhaps your expectation is that a repository-side path in the conflict
> description is changed by svn_client_conflict_option_set_moved_to_abspath()?
> If you do, then the API's behaviour will seem confusing indeed.

I think that the actual problem here is that calling the
svn_client_conflict_option_set_moved_to_repos_relpath() function
does not update the description (and the target path in the description
could have changed).

In other words, probably we're looking at something along these lines:

[[[
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 1782081)
+++ subversion/include/svn_client.h (working copy)
@@ -4495,6 +4495,7 @@ svn_error_t *
 svn_client_conflict_option_set_moved_to_repos_relpath(
   svn_client_conflict_option_t *option,
   int preferred_move_target_idx,
+  svn_client_ctx_t *ctx,
   apr_pool_t *scratch_pool);

 /**
Index: subversion/libsvn_client/conflicts.c
===================================================================
--- subversion/libsvn_client/conflicts.c (revision 1782081)
+++ subversion/libsvn_client/conflicts.c (working copy)
@@ -9414,6 +9414,7 @@ svn_error_t *
 svn_client_conflict_option_set_moved_to_repos_relpath(
   svn_client_conflict_option_t *option,
   int preferred_move_target_idx,
+  svn_client_ctx_t *ctx,
   apr_pool_t *scratch_pool)
 {
   svn_client_conflict_t *conflict = option->conflict;
@@ -9466,6 +9467,14 @@ svn_client_conflict_option_set_moved_to_repos_relp
       if (strcmp(move_target_repos_relpath, repos_relpath) == 0)
         {
           details->move_target_repos_relpath = repos_relpath;
+          /* Update option description. */
+          SVN_ERR(describe_incoming_move_merge_conflict_option(
+                    &option->description,
+                    conflict, ctx,
+                    details,
+                    conflict->pool,
+                    scratch_pool));
+
           return SVN_NO_ERROR;
         }
     }
Index: subversion/svn/conflict-callbacks.c
===================================================================
--- subversion/svn/conflict-callbacks.c (revision 1782081)
+++ subversion/svn/conflict-callbacks.c (working copy)
@@ -1751,16 +1751,9 @@ handle_tree_conflict(svn_boolean_t *resolved,
           if (conflict_option)
             {
               SVN_ERR(svn_client_conflict_option_set_moved_to_repos_relpath(
-                        conflict_option, preferred_move_target_idx, iterpool));
+                        conflict_option, preferred_move_target_idx,
+                        ctx, iterpool));

-              /* Update option description. */
-              SVN_ERR(build_tree_conflict_options(
-                        &tree_conflict_options,
-                        &possible_moved_to_repos_relpaths,
-                        &possible_moved_to_abspaths,
-                        NULL, conflict, ctx,
-                        scratch_pool, scratch_pool));
-
               /* Update conflict description. */
               SVN_ERR(svn_client_conflict_tree_get_description(
                        &incoming_change_description, &local_change_description,
]]]


Regards,
Evgeny Kotkov

Re: wish for new API or extended one

Posted by Stefan Kueng <to...@gmail.com>.

On 07.02.2017 21:34, Stefan Sperling wrote:
> On Tue, Feb 07, 2017 at 08:17:41PM +0100, Stefan Kueng wrote:
>> I figured out the problem:
>> to get updated descriptions, I have to call
>> svn_client_conflict_tree_get_resolution_options() again to get new updated
>> options. I can't just get the description from the option I set the move
>> target index to.
>>
>> this won't work:
>> svn_client_conflict_option_set_moved_to_abspath(opt, j, m_pctx,
>> scratchpool);
>>
>> label = svn_client_conflict_option_get_label(opt, scratchpool);
>> description = svn_client_conflict_option_get_description(opt, scratchpool);
>>
>>
>> but this works:
>> svn_client_conflict_option_set_moved_to_abspath(opt, j, m_pctx,
>> scratchpool);
>>
>> // now get the resolution options again, so the label/description is
>> // properly updated.
>> apr_array_header_t *opts;
>> SVNTRACE(
>>     Err = svn_client_conflict_tree_get_resolution_options(&opts, m_conflict,
>>                                                           m_pctx,
>> result.GetPool(), scratchpool),
>>     path
>> );
>> auto o = svn_client_conflict_option_find_by_id(opts, id);
>>
>> label = svn_client_conflict_option_get_label(o, scratchpool);
>> description = svn_client_conflict_option_get_description(o, scratchpool);
>>
>>
>> maybe the docs should be updated to say that a call to
>> svn_client_conflict_tree_get_resolution_options is required to get updated
>> strings?
>
> I'm guessing that you are mixing up repository paths and working copy paths.
> Perhaps your expectation is that a repository-side path in the conflict
> description is changed by svn_client_conflict_option_set_moved_to_abspath()?
> If you do, then the API's behaviour will seem confusing indeed.
>
> There are two kinds of paths which can be selected:
>
> svn_client_conflict_option_set_moved_to_repos_relpath() selects the
> repository path which corresponds to the move target. This is the important
> path to worry about. In the example I gave earlier, this selects between
> '^/trunk/alpha2' and '^/trunk/alpha3'.
> Your examples don't seem to be calling this function.

Sorry, my mistake: I copied the wrong part of the code above. Here's 
what I'm doing now:


apr_array_header_t *possible_moved_to_repos_relpaths = nullptr;
Err = svn_client_conflict_option_get_moved_to_repos_relpath_candidates
         (&possible_moved_to_repos_relpaths, opt, result.GetPool(), 
scratchpool);

if ((Err == nullptr) && possible_moved_to_repos_relpaths &&
                         (possible_moved_to_repos_relpaths->nelts > 1))
{
     for (int j = 0; j < possible_moved_to_repos_relpaths->nelts; ++j)
     {
         svn_client_conflict_option_set_moved_to_repos_relpath(opt, j,
                                                               scratchpool);

         // now get the resolution options again, so the 
label/description is
         // properly updated.
         // Note: without this, the label and description text is NOT 
updated!
	// calling svn_client_conflict_option_get_description on 'opt'
	// instead of 'o' will get the existing description, not the
	// updated one, even though we set the move index on 'opt'.
         apr_array_header_t *opts;
         SVNTRACE(
             Err = svn_client_conflict_tree_get_resolution_options(&opts,
                     m_conflict, m_pctx, result.GetPool(), scratchpool),
             path
         );
         auto o = svn_client_conflict_option_find_by_id(opts, id);

         if (0 == nullptr)
             continue;

         label = svn_client_conflict_option_get_label(o, scratchpool);
         description = svn_client_conflict_option_get_description(o,
                                                         scratchpool);

         result.push_back(std::unique_ptr<SVNConflictOption>(new
                                           SVNConflictOption(o, id,
             CUnicodeUtils::GetUnicode(label),
             CUnicodeUtils::GetUnicode(description), -1, j)));
         bResultAdded = true;
     }
}


But as I mentioned: without getting the options again with 
svn_client_conflict_tree_get_resolution_options, the strings are not 
updated. If you look at the svn code: the 
svn_client_conflict_option_set_moved_to_repos_relpath call sets the 
index to the options object, but does *not* update the strings, and 
svn_client_conflict_option_get_description just returns the already 
created string.

Stefan


-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Feb 07, 2017 at 08:17:41PM +0100, Stefan Kueng wrote:
> I figured out the problem:
> to get updated descriptions, I have to call
> svn_client_conflict_tree_get_resolution_options() again to get new updated
> options. I can't just get the description from the option I set the move
> target index to.
> 
> this won't work:
> svn_client_conflict_option_set_moved_to_abspath(opt, j, m_pctx,
> scratchpool);
> 
> label = svn_client_conflict_option_get_label(opt, scratchpool);
> description = svn_client_conflict_option_get_description(opt, scratchpool);
> 
> 
> but this works:
> svn_client_conflict_option_set_moved_to_abspath(opt, j, m_pctx,
> scratchpool);
> 
> // now get the resolution options again, so the label/description is
> // properly updated.
> apr_array_header_t *opts;
> SVNTRACE(
>     Err = svn_client_conflict_tree_get_resolution_options(&opts, m_conflict,
>                                                           m_pctx,
> result.GetPool(), scratchpool),
>     path
> );
> auto o = svn_client_conflict_option_find_by_id(opts, id);
> 
> label = svn_client_conflict_option_get_label(o, scratchpool);
> description = svn_client_conflict_option_get_description(o, scratchpool);
> 
> 
> maybe the docs should be updated to say that a call to
> svn_client_conflict_tree_get_resolution_options is required to get updated
> strings?

I'm guessing that you are mixing up repository paths and working copy paths.
Perhaps your expectation is that a repository-side path in the conflict
description is changed by svn_client_conflict_option_set_moved_to_abspath()?
If you do, then the API's behaviour will seem confusing indeed.

There are two kinds of paths which can be selected:

svn_client_conflict_option_set_moved_to_repos_relpath() selects the
repository path which corresponds to the move target. This is the important
path to worry about. In the example I gave earlier, this selects between
'^/trunk/alpha2' and '^/trunk/alpha3'.
Your examples don't seem to be calling this function.

Because of copies and because of switched subtrees, multiple representations
of a given repository relpath can exist in the working copy. Once the
repository path is selected, you can choose a corresponding path in the
working copy with svn_client_conflict_option_set_moved_to_abspath().
In my example, this will be 'alpha2' if the selected repository path is
'^/trunk/alpha2', and it will be 'alpha3' if the selected repository path
is '^/trunk/alpha3'. So in my example there is only one possible working
copy path, and its value depends on the selected repostiory path.

If this is still unclear, please show the exact text of the descriptions you
are seeing, so I can see the full paths involved and I can find any faulty
description texts in the source code. And please also explain which path
is wrong and what it should be instead. Just to avoid misunderstandings.

It also seems clear that the API documentation needs to be improved.
I am very open to suggestions, and will definitely try to improve it when
this dicussion is resolved! Please keep in mind that you are effectively
beta testing the API documentation, which I am very grateful for :)

Hope this helps,
Stefan

Re: wish for new API or extended one

Posted by Stefan Kueng <to...@gmail.com>.

On 07.02.2017 09:37, Stefan Sperling wrote:
> On Mon, Feb 06, 2017 at 07:54:46PM +0100, Stefan Kueng wrote:
>>
>>
>> On 06.02.2017 13:19, Stefan Sperling wrote:
>>> On Sun, Feb 05, 2017 at 03:23:33PM +0100, Stefan Kng wrote:
>>>> Still doesn't work for me.
>>>>
>>>> svn_client_conflict_option_set_moved_to_repos_relpath(opt, j, scratchpool);
>>>> label = svn_client_conflict_option_get_label(opt, scratchpool);
>>>> description = svn_client_conflict_option_get_description(opt, scratchpool);
>>>>
>>>> setting the moved-to path and then getting the description again does not
>>>> update the description nor the label.
>>>>
>>>> Stefan
>>>
>>> Yes, there were still some bugs to fix. As of r1781889, it works as expected
>>> for me. See the transcript below. I have added annotations in square brackets
>>> like this: [[ ... ]] so you can more easily follow which behaviour I am
>>> focussing on.
>>
>> [snip]
>>
>> Thanks for the info, but I think we're not talking about the same thing.
>> What works now is that the tree description gets updated.
>> But what I want to have updated is the tree *option* description:
>>
>> svn_client_conflict_option_get_description vs.
>> svn_client_conflict_tree_get_description
>>
>> I'd like to have the option description updated with the new path, because
>> that's what I want to use in the button text.
>>
>> Stefan
>
> We are starting to go in circles.
>
> The option description is indeed being updated in my example:
>
> Before:
>
>   (m)  - move and merge local changes from 'alpha' into 'alpha2'
>
> After:
>
>   (m)  - move and merge local changes from 'alpha' into 'alpha3'
>
> Please show me exactly which string you are referring to.
> Best is probably to show me the output you see vs the output you expect.
> Otherwise I won't understand where I should start looking for the problem.

I figured out the problem:
to get updated descriptions, I have to call 
svn_client_conflict_tree_get_resolution_options() again to get new 
updated options. I can't just get the description from the option I set 
the move target index to.

this won't work:
svn_client_conflict_option_set_moved_to_abspath(opt, j, m_pctx, 
scratchpool);

label = svn_client_conflict_option_get_label(opt, scratchpool);
description = svn_client_conflict_option_get_description(opt, scratchpool);


but this works:
svn_client_conflict_option_set_moved_to_abspath(opt, j, m_pctx, 
scratchpool);

// now get the resolution options again, so the label/description is
// properly updated.
apr_array_header_t *opts;
SVNTRACE(
     Err = svn_client_conflict_tree_get_resolution_options(&opts, 
m_conflict,
                                                           m_pctx, 
result.GetPool(), scratchpool),
     path
);
auto o = svn_client_conflict_option_find_by_id(opts, id);

label = svn_client_conflict_option_get_label(o, scratchpool);
description = svn_client_conflict_option_get_description(o, scratchpool);


maybe the docs should be updated to say that a call to 
svn_client_conflict_tree_get_resolution_options is required to get 
updated strings?

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Feb 06, 2017 at 07:54:46PM +0100, Stefan Kueng wrote:
> 
> 
> On 06.02.2017 13:19, Stefan Sperling wrote:
> > On Sun, Feb 05, 2017 at 03:23:33PM +0100, Stefan Kng wrote:
> > > Still doesn't work for me.
> > > 
> > > svn_client_conflict_option_set_moved_to_repos_relpath(opt, j, scratchpool);
> > > label = svn_client_conflict_option_get_label(opt, scratchpool);
> > > description = svn_client_conflict_option_get_description(opt, scratchpool);
> > > 
> > > setting the moved-to path and then getting the description again does not
> > > update the description nor the label.
> > > 
> > > Stefan
> > 
> > Yes, there were still some bugs to fix. As of r1781889, it works as expected
> > for me. See the transcript below. I have added annotations in square brackets
> > like this: [[ ... ]] so you can more easily follow which behaviour I am
> > focussing on.
> 
> [snip]
> 
> Thanks for the info, but I think we're not talking about the same thing.
> What works now is that the tree description gets updated.
> But what I want to have updated is the tree *option* description:
> 
> svn_client_conflict_option_get_description vs.
> svn_client_conflict_tree_get_description
> 
> I'd like to have the option description updated with the new path, because
> that's what I want to use in the button text.
> 
> Stefan

We are starting to go in circles.

The option description is indeed being updated in my example:

Before:

  (m)  - move and merge local changes from 'alpha' into 'alpha2'

After:

  (m)  - move and merge local changes from 'alpha' into 'alpha3'

Please show me exactly which string you are referring to.
Best is probably to show me the output you see vs the output you expect.
Otherwise I won't understand where I should start looking for the problem.

Re: wish for new API or extended one

Posted by Stefan Kueng <to...@gmail.com>.

On 06.02.2017 13:19, Stefan Sperling wrote:
> On Sun, Feb 05, 2017 at 03:23:33PM +0100, Stefan Kng wrote:
>> Still doesn't work for me.
>>
>> svn_client_conflict_option_set_moved_to_repos_relpath(opt, j, scratchpool);
>> label = svn_client_conflict_option_get_label(opt, scratchpool);
>> description = svn_client_conflict_option_get_description(opt, scratchpool);
>>
>> setting the moved-to path and then getting the description again does not
>> update the description nor the label.
>>
>> Stefan
>
> Yes, there were still some bugs to fix. As of r1781889, it works as expected
> for me. See the transcript below. I have added annotations in square brackets
> like this: [[ ... ]] so you can more easily follow which behaviour I am
> focussing on.

[snip]

Thanks for the info, but I think we're not talking about the same thing.
What works now is that the tree description gets updated.
But what I want to have updated is the tree *option* description:

svn_client_conflict_option_get_description vs.
svn_client_conflict_tree_get_description

I'd like to have the option description updated with the new path, 
because that's what I want to use in the button text.

Stefan


-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Sperling <st...@elego.de>.
On Sun, Feb 05, 2017 at 03:23:33PM +0100, Stefan Kng wrote:
> Still doesn't work for me.
> 
> svn_client_conflict_option_set_moved_to_repos_relpath(opt, j, scratchpool);
> label = svn_client_conflict_option_get_label(opt, scratchpool);
> description = svn_client_conflict_option_get_description(opt, scratchpool);
> 
> setting the moved-to path and then getting the description again does not
> update the description nor the label.
> 
> Stefan

Yes, there were still some bugs to fix. As of r1781889, it works as expected
for me. See the transcript below. I have added annotations in square brackets
like this: [[ ... ]] so you can more easily follow which behaviour I am
focussing on.

The tested conflict situation is as follows:
A working copy at r2 with an edited file 'alpha' was updated to r3.
In this test repository, r3 looks like this:
$ svn log -v -q -r3
------------------------------------------------------------------------
r3 | stsp | 2017-02-04 19:41:28 +0100 (Sat, 04 Feb 2017)
Changed paths:
   D /trunk/alpha
   A /trunk/alpha2 (from /trunk/alpha:2)
   A /trunk/alpha3 (from /trunk/alpha:2)
------------------------------------------------------------------------


$ svn resolve
Searching tree conflict details for 'alpha' in repository:
Checking r3... done
Tree conflict on 'alpha':
File updated from r2 to r3 was moved to '^/trunk/alpha2' by stsp in r3.
A file containing uncommitted changes was found in the working copy.
Select: (p) Postpone, (r) Mark as resolved, (m) Move and merge,
        (d) Set repository move destination path, (h) Help,
        (q) Quit resolution: h

File updated from r2 to r3 was moved to '^/trunk/alpha2' by stsp in r3.
A file containing uncommitted changes was found in the working copy.

  (p)  - skip this conflict and leave it unresolved  [postpone]
  (r)  - accept current working copy state  [working]
  (m)  - move and merge local changes from 'alpha' into 'alpha2'

[[ At this point, the move target candidate is 'trunk/alpha2' and both
   the conflict description and the "m" option show this path. ]]

  (d)  - pick repository move target from list of possible targets
  (h)  - show this help (also '?')
  (q)  - postpone all remaining conflicts
Words in square brackets are the corresponding --accept option arguments.

Select: (p) Postpone, (r) Mark as resolved, (m) Move and merge,
        (d) Set repository move destination path, (h) Help,
        (q) Quit resolution: d
Possible repository destinations for moved-away 'alpha' are:
 (1): '^/trunk/alpha2'
 (2): '^/trunk/alpha3'
Only one destination can be a move; the others are copies.
Specify the correct move target path by number: 2

[[ Here, the user has chosen to use alpha3 as the move target.
   From now on, the move target candidate is 'trunk/alpha3' and both
   the conflict description and the "m" option show this path. ]]

Select: (p) Postpone, (r) Mark as resolved, (m) Move and merge,
        (d) Set repository move destination path, (h) Help,
        (q) Quit resolution: h

File updated from r2 to r3 was moved to '^/trunk/alpha3' by stsp in r3.
A file containing uncommitted changes was found in the working copy.

  (p)  - skip this conflict and leave it unresolved  [postpone]
  (r)  - accept current working copy state  [working]
  (m)  - move and merge local changes from 'alpha' into 'alpha3'
  (d)  - pick repository move target from list of possible targets
  (h)  - show this help (also '?')
  (q)  - postpone all remaining conflicts
Words in square brackets are the corresponding --accept option arguments.

Select: (p) Postpone, (r) Mark as resolved, (m) Move and merge,
        (d) Set repository move destination path, (h) Help,
        (q) Quit resolution:

Re: wish for new API or extended one

Posted by Stefan Küng <to...@gmail.com>.
On Sat, Feb 4, 2017 at 8:25 PM, Stefan Kueng <to...@gmail.com> wrote:

>
>
> On 04.02.2017 19:52, Stefan Sperling wrote:
>
>> On Sat, Feb 04, 2017 at 07:21:50PM +0100, Stefan Sperling wrote:
>>
>>> On Sat, Feb 04, 2017 at 04:57:45PM +0100, Stefan Küng wrote:
>>>
>>>> But the problem with the description not being updated still persists.
>>>>
>>>
>>> OK, I believe I see the problem.
>>> The *option* descriptions are updated.
>>> But the conflict descriptions are not. I'll see what I can do about that.
>>>
>>
>> Should be fixed by https://svn.apache.org/r1781696
>>
>
> Thanks!
> I'll try this out tomorrow - already very late here.
>
>
Still doesn't work for me.

svn_client_conflict_option_set_moved_to_repos_relpath(opt, j, scratchpool);
label = svn_client_conflict_option_get_label(opt, scratchpool);
description = svn_client_conflict_option_get_description(opt, scratchpool);

setting the moved-to path and then getting the description again does not
update the description nor the label.

Stefan

-- 
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Kueng <to...@gmail.com>.

On 04.02.2017 19:52, Stefan Sperling wrote:
> On Sat, Feb 04, 2017 at 07:21:50PM +0100, Stefan Sperling wrote:
>> On Sat, Feb 04, 2017 at 04:57:45PM +0100, Stefan Kng wrote:
>>> But the problem with the description not being updated still persists.
>>
>> OK, I believe I see the problem.
>> The *option* descriptions are updated.
>> But the conflict descriptions are not. I'll see what I can do about that.
>
> Should be fixed by https://svn.apache.org/r1781696

Thanks!
I'll try this out tomorrow - already very late here.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Sperling <st...@elego.de>.
On Sat, Feb 04, 2017 at 07:21:50PM +0100, Stefan Sperling wrote:
> On Sat, Feb 04, 2017 at 04:57:45PM +0100, Stefan Kng wrote:
> > But the problem with the description not being updated still persists.
> 
> OK, I believe I see the problem.
> The *option* descriptions are updated.
> But the conflict descriptions are not. I'll see what I can do about that.

Should be fixed by https://svn.apache.org/r1781696

But only for 'incoming delete' conflicts!

For 'local missing' conflicts we currently lack the infrastructure
to support selection of move targets. This should be fixed, too.
If somebody wants to help out, then this might be a good task to get
started in the conflict resolver code. It should be a mostly mechanical
change, using the 'incoming delete' code as an example for adding the
missing bits and pieces to the 'local missing' code.

Re: wish for new API or extended one

Posted by Stefan Sperling <st...@elego.de>.
On Sat, Feb 04, 2017 at 04:57:45PM +0100, Stefan Kng wrote:
> But the problem with the description not being updated still persists.

OK, I believe I see the problem.
The *option* descriptions are updated.
But the conflict descriptions are not. I'll see what I can do about that.

Re: wish for new API or extended one

Posted by Stefan Küng <to...@gmail.com>.
On Sat, Feb 4, 2017 at 4:41 PM, Stefan Kueng <to...@gmail.com> wrote:

>
>
> On 04.02.2017 14:12, Stefan Sperling wrote:
>
>> On Sat, Feb 04, 2017 at 01:45:10PM +0100, Stefan Kueng wrote:
>>
>>> On 04.02.2017 11:16, Stefan Sperling wrote:
>>>
>>>> On Sat, Feb 04, 2017 at 09:23:06AM +0100, Stefan Kueng wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> while trying out the new conflict resolver APIs I came upon a slight
>>>>> problem. The API svn_client_conflict_option_get_description() returns
>>>>> a
>>>>> string that can be used to show to the user as a choice - in TSVN that
>>>>> would
>>>>> be the text on a two-line button (the 'label' is the first line, the
>>>>> 'description' the second line).
>>>>>
>>>>> The problem I'm having now is that the description is always the same,
>>>>> even
>>>>> if there are multiple 'moved-to-candidates' available. And if we use
>>>>> another
>>>>> than the first moved-to-candidate, the description mentions the wrong
>>>>> path.
>>>>>
>>>>
>>>> So you're calling svn_client_conflict_option_set
>>>> _moved_to_repos_relpath()
>>>> and if you then get the description again the path isn't updated?
>>>> That would be a bug.
>>>>
>>>
> So I tried exactly that, and for me the description is not updated. It
> returns the same description no matter what move path index I set: the
> description always mentions the first path.
>
> But fetching the path with
> const char * p = APR_ARRAY_IDX(possible_moved_to_repos_relpaths, j, const
> char *);
> shows a different path.
>
> Another thing that confuses me:
> After calling get_details(), the description mentions which path is the
> reason for the conflict. So why does svn_client_conflict_option_get
> _moved_to_repos_relpath_candidates still get me a list of paths to chose
> from if the path is already known?
> I'm a little bit confused...
>

Sorry, confusion fixed. After going through the code a little bit I now
understand why.

But the problem with the description not being updated still persists.

Stefan

-- 
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Kueng <to...@gmail.com>.

On 04.02.2017 14:12, Stefan Sperling wrote:
> On Sat, Feb 04, 2017 at 01:45:10PM +0100, Stefan Kueng wrote:
>> On 04.02.2017 11:16, Stefan Sperling wrote:
>>> On Sat, Feb 04, 2017 at 09:23:06AM +0100, Stefan Kueng wrote:
>>>> Hi,
>>>>
>>>> while trying out the new conflict resolver APIs I came upon a slight
>>>> problem. The API svn_client_conflict_option_get_description() returns a
>>>> string that can be used to show to the user as a choice - in TSVN that would
>>>> be the text on a two-line button (the 'label' is the first line, the
>>>> 'description' the second line).
>>>>
>>>> The problem I'm having now is that the description is always the same, even
>>>> if there are multiple 'moved-to-candidates' available. And if we use another
>>>> than the first moved-to-candidate, the description mentions the wrong path.
>>>
>>> So you're calling svn_client_conflict_option_set_moved_to_repos_relpath()
>>> and if you then get the description again the path isn't updated?
>>> That would be a bug.

So I tried exactly that, and for me the description is not updated. It 
returns the same description no matter what move path index I set: the 
description always mentions the first path.

But fetching the path with
const char * p = APR_ARRAY_IDX(possible_moved_to_repos_relpaths, j, 
const char *);
shows a different path.

Another thing that confuses me:
After calling get_details(), the description mentions which path is the 
reason for the conflict. So why does 
svn_client_conflict_option_get_moved_to_repos_relpath_candidates still 
get me a list of paths to chose from if the path is already known?
I'm a little bit confused...

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Kueng <to...@gmail.com>.

On 04.02.2017 14:12, Stefan Sperling wrote:
> On Sat, Feb 04, 2017 at 01:45:10PM +0100, Stefan Kueng wrote:
>> On 04.02.2017 11:16, Stefan Sperling wrote:
>>> On Sat, Feb 04, 2017 at 09:23:06AM +0100, Stefan Kueng wrote:
>>>> Hi,
>>>>
>>>> while trying out the new conflict resolver APIs I came upon a slight
>>>> problem. The API svn_client_conflict_option_get_description() returns a
>>>> string that can be used to show to the user as a choice - in TSVN that would
>>>> be the text on a two-line button (the 'label' is the first line, the
>>>> 'description' the second line).
>>>>
>>>> The problem I'm having now is that the description is always the same, even
>>>> if there are multiple 'moved-to-candidates' available. And if we use another
>>>> than the first moved-to-candidate, the description mentions the wrong path.
>>>
>>> So you're calling svn_client_conflict_option_set_moved_to_repos_relpath()
>>> and if you then get the description again the path isn't updated?
>>> That would be a bug.
>>
>> No, I'm not setting the path to the options object yet: I do that *after*
>> the user clicks the corresponding button.
>> Is it allowed to call
>> svn_client_conflict_option_set_moved_to_repos_relpath() multiple times on
>> the same options object? The docs don't mention that so I assumed no.
>> Also, is it possible to 'remove' such a set path again? Just in case the
>> user wants to do another action than setting a path.
>>
>> If all answers are "yes", then I can just set the path, get the description
>> and then later set another path if necessary before resolving the conflict.
>>
>> Stefan
>
> The idea is to just call it as many times as you want to.
> It just sets an internal index that indicates which moved-to-path to use
> for descriptions and resolution from now on.
>
> You cannot 'remove' the path. Currently, if a move is detected we force
> the user to choose a move target. Perhaps the API should account for the
> possibility where the user decides that this is not a move after all?
> With the current API you could work around this limitation by discarding
> the conflict object, then get a new one and do not call get_details() on it.
> This new conflict will offer the "accept/ignore deletion" options only.
> But that's not very pretty.

But even if you force the user to chose a move path, the options 
"Postpone" and "Mark as resolved/accept current working copy state" are 
still available?
What happens if you set a move path index but then decide on plain "Mark 
as resolved"?

As for geting a new conflict object: doesn't this ask for a 
svn_client_tree_conflict_object_duplicate() API?

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Kueng <to...@gmail.com>.

On 04.02.2017 14:24, Stefan Sperling wrote:
> On Sat, Feb 04, 2017 at 02:12:45PM +0100, Stefan Sperling wrote:
>> The idea is to just call it as many times as you want to.
>> It just sets an internal index that indicates which moved-to-path to use
>> for descriptions and resolution from now on.
>
> See https://svn.apache.org/r1781667 -- is this better?
>

much better, yes.
Thanks!

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Sperling <st...@elego.de>.
On Sat, Feb 04, 2017 at 02:12:45PM +0100, Stefan Sperling wrote:
> The idea is to just call it as many times as you want to.
> It just sets an internal index that indicates which moved-to-path to use
> for descriptions and resolution from now on.

See https://svn.apache.org/r1781667 -- is this better?

Re: wish for new API or extended one

Posted by Stefan Sperling <st...@elego.de>.
On Sat, Feb 04, 2017 at 01:45:10PM +0100, Stefan Kueng wrote:
> On 04.02.2017 11:16, Stefan Sperling wrote:
> > On Sat, Feb 04, 2017 at 09:23:06AM +0100, Stefan Kueng wrote:
> > > Hi,
> > > 
> > > while trying out the new conflict resolver APIs I came upon a slight
> > > problem. The API svn_client_conflict_option_get_description() returns a
> > > string that can be used to show to the user as a choice - in TSVN that would
> > > be the text on a two-line button (the 'label' is the first line, the
> > > 'description' the second line).
> > > 
> > > The problem I'm having now is that the description is always the same, even
> > > if there are multiple 'moved-to-candidates' available. And if we use another
> > > than the first moved-to-candidate, the description mentions the wrong path.
> > 
> > So you're calling svn_client_conflict_option_set_moved_to_repos_relpath()
> > and if you then get the description again the path isn't updated?
> > That would be a bug.
> 
> No, I'm not setting the path to the options object yet: I do that *after*
> the user clicks the corresponding button.
> Is it allowed to call
> svn_client_conflict_option_set_moved_to_repos_relpath() multiple times on
> the same options object? The docs don't mention that so I assumed no.
> Also, is it possible to 'remove' such a set path again? Just in case the
> user wants to do another action than setting a path.
> 
> If all answers are "yes", then I can just set the path, get the description
> and then later set another path if necessary before resolving the conflict.
> 
> Stefan

The idea is to just call it as many times as you want to.
It just sets an internal index that indicates which moved-to-path to use
for descriptions and resolution from now on.

You cannot 'remove' the path. Currently, if a move is detected we force
the user to choose a move target. Perhaps the API should account for the
possibility where the user decides that this is not a move after all?
With the current API you could work around this limitation by discarding
the conflict object, then get a new one and do not call get_details() on it.
This new conflict will offer the "accept/ignore deletion" options only.
But that's not very pretty.

Re: wish for new API or extended one

Posted by Stefan Kueng <to...@gmail.com>.

On 04.02.2017 11:16, Stefan Sperling wrote:
> On Sat, Feb 04, 2017 at 09:23:06AM +0100, Stefan Kueng wrote:
>> Hi,
>>
>> while trying out the new conflict resolver APIs I came upon a slight
>> problem. The API svn_client_conflict_option_get_description() returns a
>> string that can be used to show to the user as a choice - in TSVN that would
>> be the text on a two-line button (the 'label' is the first line, the
>> 'description' the second line).
>>
>> The problem I'm having now is that the description is always the same, even
>> if there are multiple 'moved-to-candidates' available. And if we use another
>> than the first moved-to-candidate, the description mentions the wrong path.
>
> So you're calling svn_client_conflict_option_set_moved_to_repos_relpath()
> and if you then get the description again the path isn't updated?
> That would be a bug.

No, I'm not setting the path to the options object yet: I do that 
*after* the user clicks the corresponding button.
Is it allowed to call 
svn_client_conflict_option_set_moved_to_repos_relpath() multiple times 
on the same options object? The docs don't mention that so I assumed no.
Also, is it possible to 'remove' such a set path again? Just in case the 
user wants to do another action than setting a path.

If all answers are "yes", then I can just set the path, get the 
description and then later set another path if necessary before 
resolving the conflict.

Stefan


-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net

Re: wish for new API or extended one

Posted by Stefan Sperling <st...@elego.de>.
On Sat, Feb 04, 2017 at 09:23:06AM +0100, Stefan Kueng wrote:
> Hi,
> 
> while trying out the new conflict resolver APIs I came upon a slight
> problem. The API svn_client_conflict_option_get_description() returns a
> string that can be used to show to the user as a choice - in TSVN that would
> be the text on a two-line button (the 'label' is the first line, the
> 'description' the second line).
> 
> The problem I'm having now is that the description is always the same, even
> if there are multiple 'moved-to-candidates' available. And if we use another
> than the first moved-to-candidate, the description mentions the wrong path.

So you're calling svn_client_conflict_option_set_moved_to_repos_relpath()
and if you then get the description again the path isn't updated?
That would be a bug.