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 Sperling <st...@elego.de> on 2008/11/22 10:59:34 UTC
Re: svn commit: r34329 - in branches/tc_url_rev/subversion:
include libsvn_wc svn
On Fri, Nov 21, 2008 at 07:37:54PM -0800, neels@tigris.org wrote:
> Author: neels
> Date: Fri Nov 21 19:37:54 2008
> New Revision: 34329
>
> Log:
> Fix some confusion with "incoming", "local", "older" and "their" versions.
Ouch, sorry about that :/
Some comments inline.
> Include the operation that caused the tree-conflict in abbreviated
> info output.
>
> Show a proper "their" path on some delete cases (in update code).
>
> * subversion/include/svn_wc.h
> (svn_wc_operation_str):
> New helper function that returns a string for an svn_wc_operation_t.
> ### TODO: really go public?
>
> * subversion/libsvn_wc/util.c
> (svn_wc_operation_str):
> New helper function that returns a string for an svn_wc_operation_t.
>
> * subversion/libsvn_wc/update_editor.c
> (do_entry_deletion):
> New argument THEIR_PATH propagated from delete_entry(), for passing
> it to check_tree_conflict().
> (delete_entry): Fill in a THEIR_PATH for do_entry_deletion().
> (close_edit, complete_directory): Just pass NULL for THEIR_PATH. ### TODO
>
> * subversion/svn/info-cmd.c
> (print_info): Don't take older/their as local/incoming. Capitalize output.
>
> * subversion/svn/tree-conflicts.c
> (select_action, select_reason): Rename these...
> (action_str, reason_str): ...to these
>
> Modified:
> branches/tc_url_rev/subversion/include/svn_wc.h
> branches/tc_url_rev/subversion/libsvn_wc/update_editor.c
> branches/tc_url_rev/subversion/libsvn_wc/util.c
> branches/tc_url_rev/subversion/svn/info-cmd.c
> branches/tc_url_rev/subversion/svn/tree-conflicts.c
>
> Modified: branches/tc_url_rev/subversion/include/svn_wc.h
> URL: http://svn.collab.net/viewvc/svn/branches/tc_url_rev/subversion/include/svn_wc.h?pathrev=34329&r1=34328&r2=34329
> ==============================================================================
> --- branches/tc_url_rev/subversion/include/svn_wc.h Fri Nov 21 15:55:47 2008 (r34328)
> +++ branches/tc_url_rev/subversion/include/svn_wc.h Fri Nov 21 19:37:54 2008 (r34329)
> @@ -1167,6 +1167,12 @@ typedef enum svn_wc_operation_t
>
> } svn_wc_operation_t;
>
> +/** Provides a human redable string for a given @a operation.
> + * @since New in 1.6.
> + */
> +const char *
> +svn_wc_operation_str(svn_wc_operation_t operation, apr_pool_t *pool);
> +
Why not call it svn_wc_operation_str_human_readable()?
We may want an XML-compatible version of this function, too.
> /** Info about one of the conflicting versions of a node. Each field may
> * have its respective null/invalid/unknown value if the corresponding
>
> Modified: branches/tc_url_rev/subversion/libsvn_wc/update_editor.c
> URL: http://svn.collab.net/viewvc/svn/branches/tc_url_rev/subversion/libsvn_wc/update_editor.c?pathrev=34329&r1=34328&r2=34329
> ==============================================================================
> --- branches/tc_url_rev/subversion/libsvn_wc/update_editor.c Fri Nov 21 15:55:47 2008 (r34328)
> +++ branches/tc_url_rev/subversion/libsvn_wc/update_editor.c Fri Nov 21 19:37:54 2008 (r34329)
> @@ -556,6 +556,7 @@ static svn_error_t *
> do_entry_deletion(struct edit_baton *eb,
> const char *parent_path,
> const char *path,
> + const char *their_path,
> int *log_number,
> apr_pool_t *pool);
>
> @@ -610,7 +611,7 @@ complete_directory(struct edit_baton *eb
> if (!target_access && entry->kind == svn_node_dir)
> {
> int log_number = 0;
> - SVN_ERR(do_entry_deletion(eb, eb->anchor, eb->target,
> + SVN_ERR(do_entry_deletion(eb, eb->anchor, eb->target, NULL,
> &log_number, pool));
> }
> else
> @@ -1625,6 +1626,7 @@ static svn_error_t *
> do_entry_deletion(struct edit_baton *eb,
> const char *parent_path,
> const char *path,
> + const char *their_path,
> int *log_number,
> apr_pool_t *pool)
> {
> @@ -1677,7 +1679,7 @@ do_entry_deletion(struct edit_baton *eb,
> SVN_ERR(check_tree_conflict(&tree_conflict, eb, log_item, full_path,
> entry, adm_access,
> svn_wc_conflict_action_delete,
> - svn_node_none, NULL, pool));
> + svn_node_none, their_path, pool));
>
> if (tree_conflict != NULL)
> {
> @@ -1821,11 +1823,13 @@ delete_entry(const char *path,
> apr_pool_t *pool)
> {
> struct dir_baton *pb = parent_baton;
> -
> - SVN_ERR(check_path_under_root(pb->path, svn_path_basename(path, pool),
> - pool));
> - return do_entry_deletion(pb->edit_baton, pb->path, path, &pb->log_number,
> - pool);
> + const char *path_basename = svn_path_basename(path, pool);
> + const char *their_path = svn_path_url_add_component(pb->new_URL,
> + path_basename, pool);
> +
> + SVN_ERR(check_path_under_root(pb->path, path_basename, pool));
> + return do_entry_deletion(pb->edit_baton, pb->path, path, their_path,
> + &pb->log_number, pool);
> }
>
>
> @@ -4057,7 +4061,8 @@ close_edit(void *edit_baton,
> pretend that the editor deleted the entry. The helper function
> do_entry_deletion() will take care of the necessary steps. */
> if ((*eb->target) && (svn_wc__adm_missing(eb->adm_access, target_path)))
> - SVN_ERR(do_entry_deletion(eb, eb->anchor, eb->target, &log_number, pool));
> + SVN_ERR(do_entry_deletion(eb, eb->anchor, eb->target, NULL,
> + &log_number, pool));
>
> /* The editor didn't even open the root; we have to take care of
> some cleanup stuffs. */
>
> Modified: branches/tc_url_rev/subversion/libsvn_wc/util.c
> URL: http://svn.collab.net/viewvc/svn/branches/tc_url_rev/subversion/libsvn_wc/util.c?pathrev=34329&r1=34328&r2=34329
> ==============================================================================
> --- branches/tc_url_rev/subversion/libsvn_wc/util.c Fri Nov 21 15:55:47 2008 (r34328)
> +++ branches/tc_url_rev/subversion/libsvn_wc/util.c Fri Nov 21 19:37:54 2008 (r34329)
> @@ -29,6 +29,7 @@
> #include "svn_dirent_uri.h"
> #include "svn_path.h"
> #include "wc.h" /* just for prototypes of things in this .c file */
> +#include "tree_conflicts.h" /* just for the SVN_WC__OPERATION* defines */
> #include "private/svn_wc_private.h"
>
> #include "svn_private_config.h"
> @@ -406,3 +407,18 @@ svn_wc__conflict_version_dup(const svn_w
>
> return new_version;
> }
> +
> +const char *
> +svn_wc_operation_str(svn_wc_operation_t operation, apr_pool_t *pool)
> +{
> + switch(operation){
> + case svn_wc_operation_update:
> + return SVN_WC__OPERATION_UPDATE;
> + case svn_wc_operation_switch:
> + return SVN_WC__OPERATION_SWITCH;
> + case svn_wc_operation_merge:
> + return SVN_WC__OPERATION_MERGE;
You haven't marked the above strings for translation.
> + }
> + return _("unknown operation");
> +}
> +
>
> Modified: branches/tc_url_rev/subversion/svn/info-cmd.c
> URL: http://svn.collab.net/viewvc/svn/branches/tc_url_rev/subversion/svn/info-cmd.c?pathrev=34329&r1=34328&r2=34329
> ==============================================================================
> --- branches/tc_url_rev/subversion/svn/info-cmd.c Fri Nov 21 15:55:47 2008 (r34328)
> +++ branches/tc_url_rev/subversion/svn/info-cmd.c Fri Nov 21 19:37:54 2008 (r34329)
> @@ -431,20 +431,20 @@ print_info(void *baton,
>
> if (info->tree_conflict)
> {
> - const char *desc, *incoming_version, *local_version;
> + const char *desc, *older_version, *their_version;
>
> SVN_ERR(svn_cl__get_human_readable_tree_conflict_description(
> &desc, info->tree_conflict, pool));
> - incoming_version =
> - svn_cl__node_description(info->tree_conflict->their_version, pool);
> - local_version =
> + older_version =
> svn_cl__node_description(info->tree_conflict->older_version, pool);
> + their_version =
> + svn_cl__node_description(info->tree_conflict->their_version, pool);
>
> svn_cmdline_printf(pool,
> _("Tree conflict: %s\n"
> - " incoming version: %s\n"
> - " local version: %s"),
> - desc, incoming_version, local_version);
> + " Older version: %s\n"
> + " Their version: %s"),
> + desc, older_version, their_version);
I'm wondering why we use "older" and "their" for merge.
It only makes sense for update.
In case of merge, wouldn't it be more straightforward to say
"merge-right" and "merge-left"?
> }
>
> /* Print extra newline separator. */
>
> Modified: branches/tc_url_rev/subversion/svn/tree-conflicts.c
> URL: http://svn.collab.net/viewvc/svn/branches/tc_url_rev/subversion/svn/tree-conflicts.c?pathrev=34329&r1=34328&r2=34329
> ==============================================================================
This hunk below should have been done on trunk, because
it's got nothing do to with the topic of the branch.
Please revert this hunk on the branch.
And I'm -1 on applying it to trunk anyway, see below.
> --- branches/tc_url_rev/subversion/svn/tree-conflicts.c Fri Nov 21 15:55:47 2008 (r34328)
> +++ branches/tc_url_rev/subversion/svn/tree-conflicts.c Fri Nov 21 19:37:54 2008 (r34329)
> @@ -25,7 +25,7 @@
> #include "svn_private_config.h"
>
> static const char *
> -select_action(const svn_wc_conflict_description_t *conflict)
> +action_str(const svn_wc_conflict_description_t *conflict)
> {
> switch (conflict->action)
> {
> @@ -35,13 +35,13 @@ select_action(const svn_wc_conflict_desc
> case svn_wc_conflict_action_add:
> return _("add");
> case svn_wc_conflict_action_delete:
> - return _("delete");
> + return _("delete or move");
I don't think this is a good idea.
Part of what I did not like about the old long human readable
descriptions was that they went out of their way to explain
to users that a move may be involved.
I am now thinking we should not pretend to be smarter than we
really are. Let's just say "delete", because we really don't
know any better. And people *know* that Subversion represents
moves as delete+add. We don't hide this fact in the update/status
output either. Just saying "delete" is more consistent.
Mercurial also does the same, btw., it just says delete.
Once the working copy has been made aware of moves, we can
adjust the output without having to lie about what we know.
> }
> return NULL;
> }
>
> static const char *
> -select_reason(const svn_wc_conflict_description_t *conflict)
> +reason_str(const svn_wc_conflict_description_t *conflict)
> {
> switch (conflict->reason)
> {
> @@ -51,7 +51,7 @@ select_reason(const svn_wc_conflict_desc
> case svn_wc_conflict_reason_obstructed:
> return _("obstruction");
> case svn_wc_conflict_reason_deleted:
> - return _("delete");
> + return _("delete or move");
> case svn_wc_conflict_reason_added:
> return _("add");
> case svn_wc_conflict_reason_missing:
> @@ -68,12 +68,14 @@ svn_cl__get_human_readable_tree_conflict
> const svn_wc_conflict_description_t *conflict,
> apr_pool_t *pool)
> {
> - const char *victim_name, *action, *reason;
> + const char *victim_name, *action, *reason, *operation;
> victim_name = svn_path_basename(conflict->path, pool);
> - action = select_action(conflict);
> - reason = select_reason(conflict);
> + reason = reason_str(conflict);
> + action = action_str(conflict);
> + operation = svn_wc_operation_str(conflict->operation, pool);
> SVN_ERR_ASSERT(action && reason);
> - *desc = apr_psprintf(pool, _("incoming %s, local %s"), action, reason);
> + *desc = apr_psprintf(pool, _("local %s, incoming %s (upon %s)."),
> + reason, action, operation);
I liked it better without parentheses and full-stop, and I don't think
that printing the operation which produced the conflict really makes
much sense.
The typical user will still remember what they were doing just before
the conflicts came about. It's not like people run an update/merge and
resolve conflicts 2 weeks later.
Stefan
> return SVN_NO_ERROR;
> }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r34329 - in branches/tc_url_rev/subversion:
include libsvn_wc svn
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Nov 24, 2008 at 02:48:18AM +0100, Neels J Hofmeyr wrote:
> Arfrever Frehtes Taifersar Arahesis wrote:
> > 2008-11-23 17:09:24 Stefan Sperling napisa�(a):
> >> On Sun, Nov 23, 2008 at 04:20:39PM +0100, Arfrever Frehtes Taifersar Arahesis wrote:
> >>> 2008-11-23 13:16:24 Stefan Sperling napisa??(a):
> >>>> On Sun, Nov 23, 2008 at 02:58:06AM +0100, Neels J Hofmeyr wrote:
> >>>>> Stefan Sperling wrote:
> >>>>>> On Fri, Nov 21, 2008 at 07:37:54PM -0800, neels@tigris.org wrote:
> >>>>>>> Author: neels
> >>>>>>> Date: Fri Nov 21 19:37:54 2008
> >>>>>>> New Revision: 34329
> >>>>>>>
> >>>>>>> +const char *
> >>>>>>> +svn_wc_operation_str(svn_wc_operation_t operation, apr_pool_t *pool)
> >>>>>>> +{
> >>>>>>> + switch(operation){
> >>>>>>> + case svn_wc_operation_update:
> >>>>>>> + return SVN_WC__OPERATION_UPDATE;
> >>>>>>> + case svn_wc_operation_switch:
> >>>>>>> + return SVN_WC__OPERATION_SWITCH;
> >>>>>>> + case svn_wc_operation_merge:
> >>>>>>> + return SVN_WC__OPERATION_MERGE;
> >>>>>> You haven't marked the above strings for translation.
> >>>>> Say, do the svn commands "update", "switch", etc. get *translated*??
> >>>> oops :)
> >>>>
> >>>> Well, I suppose not, sorry.
> >>> If they are used without "svn " prefix, then they can be translated.
> >> They are used like this:
> >>
> >> $ svn status svn-files-move1-moved/branch
> >> M svn-files-move1-moved/branch
> >> A + svn-files-move1-moved/branch/alpha.moved1
> >> A + svn-files-move1-moved/branch/alpha.moved2
> >> D C svn-files-move1-moved/branch/alpha
> >> > local delete, incoming delete upon merge <-- here
> >> $ svn info svn-files-move1-moved/branch/alpha
> >> [...]
> >> Tree conflict: local delete, incoming delete upon merge <-- and here
> >> Merge left: (file) file:///home/stsp/fu/thesis/scripts/svn/svn-files-move1-moved/repos/trunk@1
> >> Merge right: (file) file:///home/stsp/fu/thesis/scripts/svn/svn-files-move1-moved/repos/trunk@3
> >>
> >> So I suppose they should be marked for translation after all?
> >
> > Yes.
> >
>
> -1 on translating these.
>
> Here, we don't mean the general term "merge", we mean explicitly the
> Subversion operation Merge. Whenever we talk in German about Subversion, we
> say "merge", not "verschmelzen". That would sound ridiculous IMHO.
We (i.e. the two of us) may be using the english version of the word
even when speaking German, but that does not mean the rest of the
world has to do so as well.
Take a look at pl.po. There are very few english words in the polish
strings, and 'merge' isn't one of them.
For example:
msgid "Merge conflict during commit"
msgstr "Konflikt ��czenia zmian w trakcie zatwierdzania"
And if you check the German translation, it does not follow our use
of the word either:
msgid "Merge conflict during commit"
msgstr "Konflikt beim Zusammenf�hren w�hrend der �bertragung"
I'd say current translation practice pretty much mandates that
these strings be translatable.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r34329 - in branches/tc_url_rev/subversion: include
libsvn_wc svn
Posted by Neels J Hofmeyr <ne...@elego.de>.
Arfrever Frehtes Taifersar Arahesis wrote:
> 2008-11-23 17:09:24 Stefan Sperling napisał(a):
>> On Sun, Nov 23, 2008 at 04:20:39PM +0100, Arfrever Frehtes Taifersar Arahesis wrote:
>>> 2008-11-23 13:16:24 Stefan Sperling napisa??(a):
>>>> On Sun, Nov 23, 2008 at 02:58:06AM +0100, Neels J Hofmeyr wrote:
>>>>> Stefan Sperling wrote:
>>>>>> On Fri, Nov 21, 2008 at 07:37:54PM -0800, neels@tigris.org wrote:
>>>>>>> Author: neels
>>>>>>> Date: Fri Nov 21 19:37:54 2008
>>>>>>> New Revision: 34329
>>>>>>>
>>>>>>> +const char *
>>>>>>> +svn_wc_operation_str(svn_wc_operation_t operation, apr_pool_t *pool)
>>>>>>> +{
>>>>>>> + switch(operation){
>>>>>>> + case svn_wc_operation_update:
>>>>>>> + return SVN_WC__OPERATION_UPDATE;
>>>>>>> + case svn_wc_operation_switch:
>>>>>>> + return SVN_WC__OPERATION_SWITCH;
>>>>>>> + case svn_wc_operation_merge:
>>>>>>> + return SVN_WC__OPERATION_MERGE;
>>>>>> You haven't marked the above strings for translation.
>>>>> Say, do the svn commands "update", "switch", etc. get *translated*??
>>>> oops :)
>>>>
>>>> Well, I suppose not, sorry.
>>> If they are used without "svn " prefix, then they can be translated.
>> They are used like this:
>>
>> $ svn status svn-files-move1-moved/branch
>> M svn-files-move1-moved/branch
>> A + svn-files-move1-moved/branch/alpha.moved1
>> A + svn-files-move1-moved/branch/alpha.moved2
>> D C svn-files-move1-moved/branch/alpha
>> > local delete, incoming delete upon merge <-- here
>> $ svn info svn-files-move1-moved/branch/alpha
>> [...]
>> Tree conflict: local delete, incoming delete upon merge <-- and here
>> Merge left: (file) file:///home/stsp/fu/thesis/scripts/svn/svn-files-move1-moved/repos/trunk@1
>> Merge right: (file) file:///home/stsp/fu/thesis/scripts/svn/svn-files-move1-moved/repos/trunk@3
>>
>> So I suppose they should be marked for translation after all?
>
> Yes.
>
-1 on translating these.
Here, we don't mean the general term "merge", we mean explicitly the
Subversion operation Merge. Whenever we talk in German about Subversion, we
say "merge", not "verschmelzen". That would sound ridiculous IMHO. Anyone
out there should know that the conflict occured when issuing the "merge"
command, not that it happend while melting stuff together. It's the main
problem I have with most translations around, that's why I always take great
care at using original English versions. It's just much clearer. Let's not
jeopardise this output like that.
~Neels
Re: svn commit: r34329 - in branches/tc_url_rev/subversion: include libsvn_wc svn
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-11-23 17:09:24 Stefan Sperling napisał(a):
> On Sun, Nov 23, 2008 at 04:20:39PM +0100, Arfrever Frehtes Taifersar Arahesis wrote:
> > 2008-11-23 13:16:24 Stefan Sperling napisa??(a):
> > > On Sun, Nov 23, 2008 at 02:58:06AM +0100, Neels J Hofmeyr wrote:
> > > > Stefan Sperling wrote:
> > > > > On Fri, Nov 21, 2008 at 07:37:54PM -0800, neels@tigris.org wrote:
> > > > >> Author: neels
> > > > >> Date: Fri Nov 21 19:37:54 2008
> > > > >> New Revision: 34329
> > > > >>
> > > > >> +const char *
> > > > >> +svn_wc_operation_str(svn_wc_operation_t operation, apr_pool_t *pool)
> > > > >> +{
> > > > >> + switch(operation){
> > > > >> + case svn_wc_operation_update:
> > > > >> + return SVN_WC__OPERATION_UPDATE;
> > > > >> + case svn_wc_operation_switch:
> > > > >> + return SVN_WC__OPERATION_SWITCH;
> > > > >> + case svn_wc_operation_merge:
> > > > >> + return SVN_WC__OPERATION_MERGE;
> > > > >
> > > > > You haven't marked the above strings for translation.
> > > >
> > > > Say, do the svn commands "update", "switch", etc. get *translated*??
> > >
> > > oops :)
> > >
> > > Well, I suppose not, sorry.
> >
> > If they are used without "svn " prefix, then they can be translated.
>
> They are used like this:
>
> $ svn status svn-files-move1-moved/branch
> M svn-files-move1-moved/branch
> A + svn-files-move1-moved/branch/alpha.moved1
> A + svn-files-move1-moved/branch/alpha.moved2
> D C svn-files-move1-moved/branch/alpha
> > local delete, incoming delete upon merge <-- here
> $ svn info svn-files-move1-moved/branch/alpha
> [...]
> Tree conflict: local delete, incoming delete upon merge <-- and here
> Merge left: (file) file:///home/stsp/fu/thesis/scripts/svn/svn-files-move1-moved/repos/trunk@1
> Merge right: (file) file:///home/stsp/fu/thesis/scripts/svn/svn-files-move1-moved/repos/trunk@3
>
> So I suppose they should be marked for translation after all?
Yes.
--
Arfrever Frehtes Taifersar Arahesis
Re: svn commit: r34329 - in branches/tc_url_rev/subversion:
include libsvn_wc svn
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Nov 23, 2008 at 04:20:39PM +0100, Arfrever Frehtes Taifersar Arahesis wrote:
> 2008-11-23 13:16:24 Stefan Sperling napisa??(a):
> > On Sun, Nov 23, 2008 at 02:58:06AM +0100, Neels J Hofmeyr wrote:
> > > Stefan Sperling wrote:
> > > > On Fri, Nov 21, 2008 at 07:37:54PM -0800, neels@tigris.org wrote:
> > > >> Author: neels
> > > >> Date: Fri Nov 21 19:37:54 2008
> > > >> New Revision: 34329
> > > >>
> > > >> +const char *
> > > >> +svn_wc_operation_str(svn_wc_operation_t operation, apr_pool_t *pool)
> > > >> +{
> > > >> + switch(operation){
> > > >> + case svn_wc_operation_update:
> > > >> + return SVN_WC__OPERATION_UPDATE;
> > > >> + case svn_wc_operation_switch:
> > > >> + return SVN_WC__OPERATION_SWITCH;
> > > >> + case svn_wc_operation_merge:
> > > >> + return SVN_WC__OPERATION_MERGE;
> > > >
> > > > You haven't marked the above strings for translation.
> > >
> > > Say, do the svn commands "update", "switch", etc. get *translated*??
> >
> > oops :)
> >
> > Well, I suppose not, sorry.
>
> If they are used without "svn " prefix, then they can be translated.
They are used like this:
$ svn status svn-files-move1-moved/branch
M svn-files-move1-moved/branch
A + svn-files-move1-moved/branch/alpha.moved1
A + svn-files-move1-moved/branch/alpha.moved2
D C svn-files-move1-moved/branch/alpha
> local delete, incoming delete upon merge <-- here
$ svn info svn-files-move1-moved/branch/alpha
[...]
Tree conflict: local delete, incoming delete upon merge <-- and here
Merge left: (file) file:///home/stsp/fu/thesis/scripts/svn/svn-files-move1-moved/repos/trunk@1
Merge right: (file) file:///home/stsp/fu/thesis/scripts/svn/svn-files-move1-moved/repos/trunk@3
So I suppose they should be marked for translation after all?
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r34329 - in branches/tc_url_rev/subversion: include libsvn_wc svn
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-11-23 13:16:24 Stefan Sperling napisał(a):
> On Sun, Nov 23, 2008 at 02:58:06AM +0100, Neels J Hofmeyr wrote:
> > Stefan Sperling wrote:
> > > On Fri, Nov 21, 2008 at 07:37:54PM -0800, neels@tigris.org wrote:
> > >> Author: neels
> > >> Date: Fri Nov 21 19:37:54 2008
> > >> New Revision: 34329
> > >>
> > >> +const char *
> > >> +svn_wc_operation_str(svn_wc_operation_t operation, apr_pool_t *pool)
> > >> +{
> > >> + switch(operation){
> > >> + case svn_wc_operation_update:
> > >> + return SVN_WC__OPERATION_UPDATE;
> > >> + case svn_wc_operation_switch:
> > >> + return SVN_WC__OPERATION_SWITCH;
> > >> + case svn_wc_operation_merge:
> > >> + return SVN_WC__OPERATION_MERGE;
> > >
> > > You haven't marked the above strings for translation.
> >
> > Say, do the svn commands "update", "switch", etc. get *translated*??
>
> oops :)
>
> Well, I suppose not, sorry.
If they are used without "svn " prefix, then they can be translated.
--
Arfrever Frehtes Taifersar Arahesis
Re: svn commit: r34329 - in branches/tc_url_rev/subversion: include
libsvn_wc svn
Posted by Neels J Hofmeyr <ne...@elego.de>.
Stefan Sperling wrote:
> OK, let's leave it in, but can we change the output to this?
>
> + *desc = apr_psprintf(pool, _("local %s, incoming %s by %s"),
> + reason, action, operation);
+1
It currently says exactly that with "upon" instead of "by". "by" is much cooler.
~Neels
Re: local %s, incoming %s, etc. (Re: svn commit: r34329 - in
branches/tc_url_rev/subversion: include libsvn_wc svn)
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Nov 23, 2008 at 07:38:17PM +0200, Daniel Shahaf wrote:
> Yellow.
*desc = apr_psprintf(pool,
_("OMG treeconflict! :-O (local := %s); "
"(incoming := !!1!!%s!!1!!) <---(=>> by %s <<=)"),
reason, action, operation);
:)
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: local %s, incoming %s, etc. (Re: svn commit: r34329 - in branches/tc_url_rev/subversion:
include libsvn_wc svn)
Posted by Neels J Hofmeyr <ne...@elego.de>.
Daniel Shahaf wrote:
> Stefan Sperling wrote on Sun, 23 Nov 2008 at 16:13 -0000:
>> On Sun, Nov 23, 2008 at 12:16:24PM +0000, Stefan Sperling wrote:
>>> On Sun, Nov 23, 2008 at 02:58:06AM +0100, Neels J Hofmeyr wrote:
>>>> I think it does. It tells people what they were trying to do, and what they
>>>> need to do to fix it. If we merge into an update-conflicted working copy,
>>>> and that merge creates more tree-conflicts... let's just leave it there.
>>> OK, let's leave it in, but can we change the output to this?
>>>
>>> + *desc = apr_psprintf(pool, _("local %s, incoming %s by %s"),
>>> + reason, action, operation);
>
>>> Anyway, we're making up the output based on gut-feeling, not human
>>> interface research guidelines. So there's not much point in arguing.
>>> Let's just try to find a form of output we're all more or less
>>> happy with.
>
> I think that it is hard (since there is a word on either side of it,
> with no visual delimiters) to quickly parse out the 'action' phrase
> (which is the interesting part (e.g., it is the only thing in the
> message that the user didn't know before the operation started)) from
> the *desc string.
>
> I'd prefer the previous formats (either "local %s, incoming %s" or the
> same with " (by %s)" appended) -- I found both of them easier to parse
> (maybe because they had more punctuation around the replaceable parts).
>
> Yellow.
LOL! I'm also +1 for yellow, but whatever.
~Neels
local %s, incoming %s, etc. (Re: svn commit: r34329 - in
branches/tc_url_rev/subversion: include libsvn_wc svn)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Sun, 23 Nov 2008 at 16:13 -0000:
> On Sun, Nov 23, 2008 at 12:16:24PM +0000, Stefan Sperling wrote:
> > On Sun, Nov 23, 2008 at 02:58:06AM +0100, Neels J Hofmeyr wrote:
> > > I think it does. It tells people what they were trying to do, and what they
> > > need to do to fix it. If we merge into an update-conflicted working copy,
> > > and that merge creates more tree-conflicts... let's just leave it there.
> >
> > OK, let's leave it in, but can we change the output to this?
> >
> > + *desc = apr_psprintf(pool, _("local %s, incoming %s by %s"),
> > + reason, action, operation);
> > Anyway, we're making up the output based on gut-feeling, not human
> > interface research guidelines. So there's not much point in arguing.
> > Let's just try to find a form of output we're all more or less
> > happy with.
>
I think that it is hard (since there is a word on either side of it,
with no visual delimiters) to quickly parse out the 'action' phrase
(which is the interesting part (e.g., it is the only thing in the
message that the user didn't know before the operation started)) from
the *desc string.
I'd prefer the previous formats (either "local %s, incoming %s" or the
same with " (by %s)" appended) -- I found both of them easier to parse
(maybe because they had more punctuation around the replaceable parts).
Yellow.
Daniel
> I'm happy with the current output :)
>
> Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r34329 - in branches/tc_url_rev/subversion:
include libsvn_wc svn
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Nov 23, 2008 at 12:16:24PM +0000, Stefan Sperling wrote:
> On Sun, Nov 23, 2008 at 02:58:06AM +0100, Neels J Hofmeyr wrote:
> > I think it does. It tells people what they were trying to do, and what they
> > need to do to fix it. If we merge into an update-conflicted working copy,
> > and that merge creates more tree-conflicts... let's just leave it there.
>
> OK, let's leave it in, but can we change the output to this?
>
> + *desc = apr_psprintf(pool, _("local %s, incoming %s by %s"),
> + reason, action, operation);
I just realised that it already looks like that, it's just
saying 'upon' instead of 'by' --> bikeshed.
> Anyway, we're making up the output based on gut-feeling, not human
> interface research guidelines. So there's not much point in arguing.
> Let's just try to find a form of output we're all more or less
> happy with.
I'm happy with the current output :)
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r34329 - in branches/tc_url_rev/subversion:
include libsvn_wc svn
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Nov 23, 2008 at 02:58:06AM +0100, Neels J Hofmeyr wrote:
> Stefan Sperling wrote:
> > On Fri, Nov 21, 2008 at 07:37:54PM -0800, neels@tigris.org wrote:
> >> Author: neels
> >> Date: Fri Nov 21 19:37:54 2008
> >> New Revision: 34329
> >>
> >> +const char *
> >> +svn_wc_operation_str(svn_wc_operation_t operation, apr_pool_t *pool)
> >> +{
> >> + switch(operation){
> >> + case svn_wc_operation_update:
> >> + return SVN_WC__OPERATION_UPDATE;
> >> + case svn_wc_operation_switch:
> >> + return SVN_WC__OPERATION_SWITCH;
> >> + case svn_wc_operation_merge:
> >> + return SVN_WC__OPERATION_MERGE;
> >
> > You haven't marked the above strings for translation.
>
> Say, do the svn commands "update", "switch", etc. get *translated*??
oops :)
Well, I suppose not, sorry.
> ...
> >> svn_cmdline_printf(pool,
> >> _("Tree conflict: %s\n"
> >> - " incoming version: %s\n"
> >> - " local version: %s"),
> >> - desc, incoming_version, local_version);
> >> + " Older version: %s\n"
> >> + " Their version: %s"),
> >> + desc, older_version, their_version);
> >
> > I'm wondering why we use "older" and "their" for merge.
> > It only makes sense for update.
> >
> > In case of merge, wouldn't it be more straightforward to say
> > "merge-right" and "merge-left"?
>
> True. I was thinking something like that but it wasn't as clear as you said
> it. Thanks.
Great.
> >> @@ -35,13 +35,13 @@ select_action(const svn_wc_conflict_desc
> >> case svn_wc_conflict_action_add:
> >> return _("add");
> >> case svn_wc_conflict_action_delete:
> >> - return _("delete");
> >> + return _("delete or move");
> >
> > I don't think this is a good idea.
> >
> > Part of what I did not like about the old long human readable
> > descriptions was that they went out of their way to explain
> > to users that a move may be involved.
> >
> > I am now thinking we should not pretend to be smarter than we
> > really are. Let's just say "delete", because we really don't
> > know any better. And people *know* that Subversion represents
> > moves as delete+add. We don't hide this fact in the update/status
> > output either. Just saying "delete" is more consistent.
> > Mercurial also does the same, btw., it just says delete.
> >
> > Once the working copy has been made aware of moves, we can
> > adjust the output without having to lie about what we know.
>
> I was thinking of users that don't know much about nothing. "What? I didn't
> *remove* that file, did I???"
>
> Then again, I don't mind it all that much. I'll revert that.
OK.
> > - *desc = apr_psprintf(pool, _("incoming %s, local %s"), action, reason);
> > + *desc = apr_psprintf(pool, _("local %s, incoming %s (upon %s)."),
> > + reason, action, operation);
> > I liked it better without parentheses and full-stop, and I don't think
>
> I like it better *with*. But whatever. I'll change it but quit bikeshedding. :)
>
> > that printing the operation which produced the conflict really makes
> > much sense.
>
> I think it does. It tells people what they were trying to do, and what they
> need to do to fix it. If we merge into an update-conflicted working copy,
> and that merge creates more tree-conflicts... let's just leave it there.
OK, let's leave it in, but can we change the output to this?
+ *desc = apr_psprintf(pool, _("local %s, incoming %s by %s"),
+ reason, action, operation);
This makes the operation visible but does not clutter the output
with parens, so nothing suggests that any one piece of information
in that line is more important than any other.
Anyway, we're making up the output based on gut-feeling, not human
interface research guidelines. So there's not much point in arguing.
Let's just try to find a form of output we're all more or less
happy with.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r34329 - in branches/tc_url_rev/subversion: include
libsvn_wc svn
Posted by Neels J Hofmeyr <ne...@elego.de>.
Stefan Sperling wrote:
> On Fri, Nov 21, 2008 at 07:37:54PM -0800, neels@tigris.org wrote:
>> Author: neels
>> Date: Fri Nov 21 19:37:54 2008
>> New Revision: 34329
>>
>> +const char *
>> +svn_wc_operation_str(svn_wc_operation_t operation, apr_pool_t *pool)
>> +{
>> + switch(operation){
>> + case svn_wc_operation_update:
>> + return SVN_WC__OPERATION_UPDATE;
>> + case svn_wc_operation_switch:
>> + return SVN_WC__OPERATION_SWITCH;
>> + case svn_wc_operation_merge:
>> + return SVN_WC__OPERATION_MERGE;
>
> You haven't marked the above strings for translation.
Say, do the svn commands "update", "switch", etc. get *translated*??
...
>> svn_cmdline_printf(pool,
>> _("Tree conflict: %s\n"
>> - " incoming version: %s\n"
>> - " local version: %s"),
>> - desc, incoming_version, local_version);
>> + " Older version: %s\n"
>> + " Their version: %s"),
>> + desc, older_version, their_version);
>
> I'm wondering why we use "older" and "their" for merge.
> It only makes sense for update.
>
> In case of merge, wouldn't it be more straightforward to say
> "merge-right" and "merge-left"?
True. I was thinking something like that but it wasn't as clear as you said
it. Thanks.
>
>> }
>>
>> /* Print extra newline separator. */
>>
>> Modified: branches/tc_url_rev/subversion/svn/tree-conflicts.c
>> URL: http://svn.collab.net/viewvc/svn/branches/tc_url_rev/subversion/svn/tree-conflicts.c?pathrev=34329&r1=34328&r2=34329
>> ==============================================================================
>
> This hunk below should have been done on trunk, because
> it's got nothing do to with the topic of the branch.
>
> Please revert this hunk on the branch.
> And I'm -1 on applying it to trunk anyway, see below.
>
>> --- branches/tc_url_rev/subversion/svn/tree-conflicts.c Fri Nov 21 15:55:47 2008 (r34328)
>> +++ branches/tc_url_rev/subversion/svn/tree-conflicts.c Fri Nov 21 19:37:54 2008 (r34329)
>> @@ -25,7 +25,7 @@
>> #include "svn_private_config.h"
>>
>> static const char *
>> -select_action(const svn_wc_conflict_description_t *conflict)
>> +action_str(const svn_wc_conflict_description_t *conflict)
>> {
>> switch (conflict->action)
>> {
>> @@ -35,13 +35,13 @@ select_action(const svn_wc_conflict_desc
>> case svn_wc_conflict_action_add:
>> return _("add");
>> case svn_wc_conflict_action_delete:
>> - return _("delete");
>> + return _("delete or move");
>
> I don't think this is a good idea.
>
> Part of what I did not like about the old long human readable
> descriptions was that they went out of their way to explain
> to users that a move may be involved.
>
> I am now thinking we should not pretend to be smarter than we
> really are. Let's just say "delete", because we really don't
> know any better. And people *know* that Subversion represents
> moves as delete+add. We don't hide this fact in the update/status
> output either. Just saying "delete" is more consistent.
> Mercurial also does the same, btw., it just says delete.
>
> Once the working copy has been made aware of moves, we can
> adjust the output without having to lie about what we know.
I was thinking of users that don't know much about nothing. "What? I didn't
*remove* that file, did I???"
Then again, I don't mind it all that much. I'll revert that.
> - *desc = apr_psprintf(pool, _("incoming %s, local %s"), action, reason);
> + *desc = apr_psprintf(pool, _("local %s, incoming %s (upon %s)."),
> + reason, action, operation);
> I liked it better without parentheses and full-stop, and I don't think
I like it better *with*. But whatever. I'll change it but quit bikeshedding. :)
> that printing the operation which produced the conflict really makes
> much sense.
I think it does. It tells people what they were trying to do, and what they
need to do to fix it. If we merge into an update-conflicted working copy,
and that merge creates more tree-conflicts... let's just leave it there.
>
> The typical user will still remember what they were doing just before
> the conflicts came about. It's not like people run an update/merge and
> resolve conflicts 2 weeks later.
And why prevent people from reading which conflict came from which
operation? It's not another line of output and we're carrying the OPERATION
information around everywhere already. What *is* your problem?
Thanks for the review!
~Neels