Hi Daniel,

Here's my previous mail as plain text (utf8).

tijn

On 2010-12-22 14:02, Daniel Shahaf wrote:
>
> Forwarding back to the list.
>
> Tijn Porcelijn wrote on Wed, Dec 22, 2010 at 01:54:07 -0800:
> > Hi Daniel,
> >
> > I guess you're right. It makes more sense to echo the external URL
> > than the local directory in the "Updated external
> > 'url://remote/path/to/somewhere' to revision %ld". Anyway, my
> > suggestion was to make sure that "some" reference to the specific url
> > was made *at all* after removing the other notice (as opposed to just
> > printing a revision number and having to guess the repository it
> > refers to).
>
> I'm still not sure I understand the issue, or why you think adding that
> would be useful.  (Not saying that it isn't useful; just that I don't
> understand why it would be.)
>
> Before you invest more time in coding, could you please give more
> concrete examples of how the current output is not satisfactory?
>
> e.g., are you trying to parse it with a script?  Or is it just that the
> information you want has scrolled offscreen and you want to repeat it
> nearer to the end of the output?
>
My personal itch is primarily that "svn up" is too verbose when using 
externals. This shows especially when there are no changes whatsoever.

For example, when I do update a normal directory and there are no 
changes (ie.: my BASE equals HEAD) I get almost no feedback: one line 
saying: "At revision 123". Now if I do the same for a directory that 
includes one svn:externals reference, I get:

   1. <<empty line>>
   2. Fetching external item into 'path/to/local/dir'
   3. External at revision 12345.
   4. <<empty line>>

For the project I'm currently working on (using 10+ externals) this 
easily fills up my screen.
In an (admittedly half-hearted) attempt I removed line 2 when doing svn 
up (but not for export and checkout). This was submitted in a different 
mail to this mailing list, using the same issue number.
Then, in below patch, I added the 'path/to/local/dir' to line 3, in an 
attempt to explain to the end-user what files the revision number is 
referring to. So, for every svn:externals entry I now get one line with 
the same output:

   1. External 'path/to/local/dir' at revision 12345.

I hope this helps explain the fix.

Tijn

>
> Thanks,
>
> Daniel
>
> > I will look into printing the remote external's path.
> >
> > Thanks, tijn
> >
> > On 2010-12-22 02:54, Daniel Shahaf wrote:
> >
> > This has a bug, when updating a file external it displays the
> > external's directory rather than the external itself.  (But maybe this
> > is a bug in the way the library generates the notifications?)
> >
> > May I ask what is the motivation for this change?  The normal
> > notifications (U   path/to/somewhere) will always immediately precede
> > the "Updated external 'path/to/somewhere' to revision %ld", so
> > repeating the external's path there seems a bit redundant.
> >
> > I haven't run 'make check'.
> >
> > Daniel
> >
> >
> > Tijn Porcelijn wrote on Fri, Nov 26, 2010 at 01:56:22 -0800:
> > > [[[
> > > Improves interaction, issue #3653: svn update should not output 
> svn:external
> > > * subversion/svn/notify.c (notify)
> > >   Add <path_local> to Externals messages
> > >   Note: po files should also be updated
> > > ]]]
> > >
> > >
> > >
> > >
> > > Hi,
> > >
> > > Here's a small patch for making svn:externals messages a bit more 
> informative. With the "Fetching external item into '<path_local>'" 
> -message removed, interpretation of svn_wc_notify_update_completed 
> messages becomes a bit less obvious. You'll see stuff like:
> > > External at revision 20
> > > External at revision 2321
> > > External at revision 1082367
> > > At revision 19
> > > The patch improves this to read:
> > > External 'third-party' at revision 20
> > > External 'snapshots' at revision 2321
> > > External 'legacy' at revision 1082367
> > > At revision 19
> > > See attached notify.c.patch, Thanks,
> > >
> > > tijn
> > >
> >
> > Content-Description: notify.c.patch
> > > Index: subversion/svn/notify.c
> > > ===================================================================
> > > --- subversion/svn/notify.c   (revision 1038983)
> > > +++ subversion/svn/notify.c   (working copy)
> > > @@ -567,44 +567,66 @@
> > >                {
> > >                  if (nb->is_export)
> > >                    {
> > > -                    if ((err = svn_cmdline_printf
> > > -                         (pool, nb->in_external
> > > -                          ? _("Exported external at revision %ld.\n")
> > > -                          : _("Exported revision %ld.\n"),
> > > -                          n->revision)))
> > > -                      goto print_error;
> > > +                    if (nb->in_external)
> > > +                      err = svn_cmdline_printf
> > > +                         (pool,
> > > +                          _("Exported external '%s' at revision 
> %ld.\n"),
> > > +                          path_local,
> > > +                          n->revision);
> > > +                    else
> > > +                      err = svn_cmdline_printf
> > > +                         (pool,
> > > +                          _("Exported revision %ld.\n"),
> > > +                          n->revision);
> > >                    }
> > >                  else if (nb->is_checkout)
> > >                    {
> > > -                    if ((err = svn_cmdline_printf
> > > -                         (pool, nb->in_external
> > > -                          ? _("Checked out external at revision 
> %ld.\n")
> > > -                          : _("Checked out revision %ld.\n"),
> > > -                          n->revision)))
> > > -                      goto print_error;
> > > +                    if (nb->in_external)
> > > +                      err = svn_cmdline_printf
> > > +                         (pool,
> > > +                          _("Checked out external '%s' at 
> revision %ld.\n"),
> > > +                          path_local,
> > > +                          n->revision);
> > > +                    else
> > > +                      err = svn_cmdline_printf
> > > +                         (pool,
> > > +                          _("Checked out revision %ld.\n"),
> > > +                          n->revision);
> > >                    }
> > >                  else
> > >                    {
> > >                      if (nb->received_some_change)
> > >                        {
> > >                          nb->received_some_change = FALSE;
> > > -                        if ((err = svn_cmdline_printf
> > > -                             (pool, nb->in_external
> > > -                              ? _("Updated external to revision 
> %ld.\n")
> > > -                              : _("Updated to revision %ld.\n"),
> > > -                              n->revision)))
> > > -                          goto print_error;
> > > +                        if (nb->in_external)
> > > +                          err = svn_cmdline_printf
> > > +                             (pool,
> > > +                              _("Updated external '%s' to 
> revision %ld.\n"),
> > > +                              path_local,
> > > +                              n->revision);
> > > +                        else
> > > +                          err = svn_cmdline_printf
> > > +                             (pool,
> > > +                              _("Updated to revision %ld.\n"),
> > > +                              n->revision);
> > >                        }
> > >                      else
> > >                        {
> > > -                        if ((err = svn_cmdline_printf
> > > -                             (pool, nb->in_external
> > > -                              ? _("External at revision %ld.\n")
> > > -                              : _("At revision %ld.\n"),
> > > -                              n->revision)))
> > > -                          goto print_error;
> > > +                        if (nb->in_external)
> > > +                          err = svn_cmdline_printf
> > > +                             (pool,
> > > +                              _("External '%s' at revision %ld.\n"),
> > > +                              path_local,
> > > +                              n->revision);
> > > +                        else
> > > +                          err = svn_cmdline_printf
> > > +                             (pool,
> > > +                              _("At revision %ld.\n"),
> > > +                              n->revision);
> > >                        }
> > >                    }
> > > +                if (err)
> > > +                  goto print_error;
> > >                }
> > >              else  /* no revision */
> > >                {
> >
> >
> >
>

Reply via email to