subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan <luke1...@posteo.de>
Subject Re: [PATCH v3] Conflict option labels
Date Thu, 13 Oct 2016 15:59:01 GMT
On 10/13/2016 5:26 PM, Patrick Steinhardt wrote:
> sion re-adds the result pool to
> `svn_client_conflict_option_get_lazel`.

> diff --git a/subversion/include/svn_client.h
> b/subversion/include/svn_client.h
> index 9bbe62b..f456c92 100644
> --- a/subversion/include/svn_client.h
> +++ b/subversion/include/svn_client.h
> @@ -4718,6 +4718,20 @@ svn_client_conflict_option_id_t
>  svn_client_conflict_option_get_id(svn_client_conflict_option_t *option);
>  
> [...]
>  
>        add_resolution_option(*options, conflict,
>          svn_client_conflict_option_merged_text,
> +        _("Mark as resolved"),
>          _("accept binary file as it appears in the working copy"),
>          resolve_text_conflict);
Not sure whether "Mark as resolved" means much to the user. Maybe
"Accept/Use current version" would be easier to get? Or maybe
"Accept/Use current" to keep the label consistent with the style of the
others.
> [...]
>  
>        add_resolution_option(*options, conflict,
>          svn_client_conflict_option_working_text,
> +        _("Reject incoming"),
>          _("reject all incoming changes for this file"),
>          resolve_text_conflict);
IMO "Reject incoming" is the inversed way to describe what's being done.
Better would be: "Accept current"?
>  
> [...]
>  
>        add_resolution_option(*options, conflict,
>          svn_client_conflict_option_working_text_where_conflicted,
> +        _("Reject conflicts"),
>          _("reject changes which conflict and accept the rest"),
>          resolve_text_conflict);
=> "Accept incoming non-conflicting changes only."?
>  
>        add_resolution_option(*options, conflict,
>          svn_client_conflict_option_merged_text,
> +        _("Mark as resolved"),
>          _("accept the file as it appears in the working copy"),
>          resolve_text_conflict);
Same as above: "Accept/Use current"
>    [...]
>  
>    add_resolution_option(*options, conflict,
>      svn_client_conflict_option_working_text,
> +    _("Mark as resolved"),
>      _("accept working copy version of entire property value"),
>      resolve_prop_conflict);
Same as above.
>  
>    add_resolution_option(*options, conflict,
>      svn_client_conflict_option_incoming_text_where_conflicted,
> -    N_("accept changes only where they conflict"),
> +    _("Accept incoming for conflicts"),
> +    _("accept incoming changes only where they conflict"),
>      resolve_prop_conflict);
The wording is a bit misleading IMO. It might be interpreted as
non-conflicting incoming changes being rejected rather than these being
merged implicitly (or am I wrong?).

Maybe better wording would be:
Label: Resolve conflicts with incoming changes
Desc: Accept incoming and resolve conflicts using the incoming changes.

>  
>    add_resolution_option(*options, conflict,
>      svn_client_conflict_option_working_text_where_conflicted,
> +    _("Reject conflicts"),
>      _("reject changes which conflict and accept the rest"),
>      resolve_prop_conflict);
Reject conflicts doesn't quite reflect what's being done. Maybe:

"Resolve conflicts with local changes."
"Accept incoming and resolve conflicts using the local changes."

> [...]
>    add_resolution_option(options, conflict,
>                         
> svn_client_conflict_option_accept_current_wc_state,
> +                        _("Mark as resolved"),
>                          _("accept current working copy state"),
>                          do_resolve_func);
See above.
>  
> @@ -7264,6 +7285,7 @@
> configure_option_update_move_destination(svn_client_conflict_t *conflict,
>        add_resolution_option(
>          options, conflict,
>          svn_client_conflict_option_update_move_destination,
> +        _("Update move destination"),
>          _("apply incoming changes to move destination"),
>          resolve_update_moved_away_node);
>      }
The label doesn't quite reflect what's being done (aka: how it's
updated). Maybe: "Apply changes to move destination"

> @@ -7298,6 +7320,7 @@ configure_option_update_raise_moved_away_children(
>        add_resolution_option(
>          options, conflict,
>          svn_client_conflict_option_update_any_moved_away_children,
> +        _("Update any moved-away children"),
>          _("prepare for updating moved-away children, if any"),
>          resolve_update_raise_moved_away);
>      }
Same as above.
> [...]
>  
>    return SVN_NO_ERROR;
> @@ -7425,7 +7448,8 @@
> configure_option_incoming_added_file_text_merge(svn_client_conflict_t
> *conflict,
>        add_resolution_option(
>          options, conflict,
>          svn_client_conflict_option_incoming_added_file_text_merge,
> -        description, resolve_merge_incoming_added_file_text_merge);
> +        _("Merge the files"), description,
> +        resolve_merge_incoming_added_file_text_merge);
>      }
Maybe better: Merge incoming file with local.
>  
>    return SVN_NO_ERROR;
> @@ -7481,6 +7505,7 @@
> configure_option_incoming_added_file_replace_and_merge(
>        add_resolution_option(
>          options, conflict,
>          svn_client_conflict_option_incoming_added_file_replace_and_merge,
> +        _("Replace and merge"),
>          description,
> resolve_merge_incoming_added_file_replace_and_merge);
>      }
TBH: I'm not sure what the behavior of this option is. It sounds like
it's the same as the previous one to me... Where's the difference?
>  
> @@ -7533,7 +7558,7 @@
> configure_option_incoming_added_dir_merge(svn_client_conflict_t *conflict,
>              scratch_pool));
>        add_resolution_option(options, conflict,
>                             
> svn_client_conflict_option_incoming_added_dir_merge,
> -                            description,
> +                            _("Merge the directories"), description,
>                              resolve_merge_incoming_added_dir_merge);
>      }
>  
> [..]
>  
>    return SVN_NO_ERROR;
> @@ -7644,8 +7669,8 @@
> configure_option_incoming_added_dir_replace_and_merge(
>        add_resolution_option(
>          options, conflict,
>          svn_client_conflict_option_incoming_added_dir_replace_and_merge,
> -        description,
> -        resolve_merge_incoming_added_dir_replace_and_merge);
> +        _("Replace and merge"),
> +        description, resolve_merge_incoming_added_dir_replace_and_merge);
>      }
Same as before. Where is this different to
svn_client_conflict_option_incoming_added_dir_merge?


Great work btw. :-)

Regards,
Stefan



Mime
View raw message