You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/12/22 01:54:35 UTC
Re: [PATCH] Issue #3653: svn update should not output svn:external
fetches if they have not been updated
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 */
> {
Re: [PATCH] Issue #3653: svn update should not output svn:external
fetches if they have not been updated
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Your quoting is broken, please send MIME=text/plain replies so we don't
have to think where my text ends and yours starts, thanks.
Tijn Porcelijn wrote on Wed, Dec 22, 2010 at 07:37:50 -0800:
> 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 see. However, I'm not sure we should make this change, since I find
the existing output format reasonable (though possibly not ideal).
In other words, I'd like to see more opinions on whether we should be
making this change. (Making the change is probably just a SMOP in
notify.c...)
> I hope this helps explain the fix.
>
> Tijn
>
>
> > Thanks,
> >
> > Daniel
> >
> > > I will look into printing the remote external's path.
> > >
Thanks!
> > > 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 */
> > > > {
> > >
> > >
> > >
>
>
Re: [PATCH] Issue #3653: svn update should not output svn:external
fetches if they have not been updated
Posted by Tijn Porcelijn <ti...@q-go.com>.
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 */
> > {
>
>
>
Re: [PATCH] Issue #3653: svn update should not output svn:external
fetches if they have not been updated
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Tijn Porcelijn wrote on Thu, Dec 23, 2010 at 00:09:32 -0800:
> Hi Daniel,
>
> Here's my previous mail as plain text (utf8).
>
Thanks, much better.
(I forgot to say that I asked for plain texts only in future mails ---
i.e., I didn't mean to ask you to re-send this one --- but thanks anyway
for doing so :))
Daniel
> 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 */
> > > > {
> > >
> > >
> > >
> >
>
Re: [PATCH] Issue #3653: svn update should not output svn:external
fetches if they have not been updated
Posted by Tijn Porcelijn <ti...@q-go.com>.
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 */
> > > {
> >
> >
> >
>
Re: [PATCH] Issue #3653: svn update should not output svn:external
fetches if they have not been updated
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
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?
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 */
> > {
>
>
>