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 Näslund <da...@longitudo.com> on 2010/04/02 10:36:53 UTC
[PATCH] v2 Fix svnversion message as follow-up to r922176
On Tue, Mar 30, 2010 at 11:04:13PM +0200, Stefan Sperling wrote:
> On Tue, Mar 30, 2010 at 10:14:42PM +0200, Daniel N�slund wrote:
> > Ping! This patch has not been reviewed!
> >
> > On Sun, Mar 14, 2010 at 09:38:15PM +0100, Daniel N�slund wrote:
> > > Hi!
> > >
> > > The 1.6 svnversion message was "'path' not versioned, and not exported".
> > > But on trunk more than one message has been changed. My first thought
> > > was that we should be backward compat in our output but if changes of
> > > those messages are ok I'm supplying one.
> > >
> > > In case we will use new messages, the help text must be updated. It
> > > talks of 'exported' but those are not used in the new messages.
> > >
> > > [[[
> > > After the changes in r922176, versioned but not yet committed files were
> > > not properly detected. Fixed now!
> > >
> > > * subversion/svnversion/main.c
> > > (main): Check for invalid rev nr for files and dirs.
> > >
> > > * subversion/tests/cmdline/svnversion_tests.py
> > > (structural_changes): New.
> > > (tests_list): Add new test.
> > > ]]]
> >
> > > Index: subversion/svnversion/main.c
> > > ===================================================================
> > > --- subversion/svnversion/main.c (revision 922931)
> > > +++ subversion/svnversion/main.c (arbetskopia)
> > > @@ -290,6 +290,17 @@
> > > return EXIT_FAILURE;
> > > }
> > >
> > > + if (res->min_rev == -1)
>
> s/-1/SVN_INVALID_REVNUM/
Fixed!
> > > + {
> > > + /* Local uncommited modifications, no revision info was found. */
> > > + SVN_INT_ERR(svn_cmdline_printf(pool, _("Local uncommitted "
> > > + "modifications, no revision "
> > > + "information found%s"),
> > > + no_newline ? "" : "\n"));
>
> "No revision information found" sounds a bit like something went
> wrong, as in, Subversion was looking for information that was
> expected to be present but it didn't find the information.
> What about "Uncommitted local addition, copy, or move%s" instead?
I agree with you (user messages is my weak spot). The only cases when we
can have no revision number but the path is under version control is for
locally added, copied or moved paths. So we tell the user about it.
Can I have your +1 for commiting?
Daniel
Re: [PATCH] v4 Fix svnversion message as follow-up to r922176
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Apr 02, 2010 at 11:06:32AM +0200, Daniel Näslund wrote:
> gstein pointed out that I should use SVN_IS_VALID_REVNUM(). Fixed!
> @@ -290,6 +290,16 @@ main(int argc, const char *argv[])
> return EXIT_FAILURE;
> }
>
> + if (SVN_IS_VALID_REVNUM(res->min_rev))
Doesn't this reverse the meaning of what you had before?
Maybe use:
if (! SVN_IS_VALID_REVNUM(res->min_rev))
or use
if (SVN_IS_INVALID_REVNUM(res->min_rev))
if there is such a macro.
> + {
> + /* Local uncommited modifications, no revision info was found. */
> + SVN_INT_ERR(svn_cmdline_printf(pool, _("Uncommitted local addition "
> + "copy, or move%s"),
> + no_newline ? "" : "\n"));
> + svn_pool_destroy(pool);
> + return EXIT_SUCCESS;
> + }
> +
> /* Build compact '123[:456]M?S?' string. */
> SVN_INT_ERR(svn_cmdline_printf(pool, "%ld", res->min_rev));
> if (res->min_rev != res->max_rev)
Re: [PATCH] v4 Fix svnversion message as follow-up to r922176
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Apr 02, 2010 at 11:06:32AM +0200, Daniel Näslund wrote:
> gstein pointed out that I should use SVN_IS_VALID_REVNUM(). Fixed!
> @@ -290,6 +290,16 @@ main(int argc, const char *argv[])
> return EXIT_FAILURE;
> }
>
> + if (SVN_IS_VALID_REVNUM(res->min_rev))
Doesn't this reverse the meaning of what you had before?
Maybe use:
if (! SVN_IS_VALID_REVNUM(res->min_rev))
or use
if (SVN_IS_INVALID_REVNUM(res->min_rev))
if there is such a macro.
> + {
> + /* Local uncommited modifications, no revision info was found. */
> + SVN_INT_ERR(svn_cmdline_printf(pool, _("Uncommitted local addition "
> + "copy, or move%s"),
> + no_newline ? "" : "\n"));
> + svn_pool_destroy(pool);
> + return EXIT_SUCCESS;
> + }
> +
> /* Build compact '123[:456]M?S?' string. */
> SVN_INT_ERR(svn_cmdline_printf(pool, "%ld", res->min_rev));
> if (res->min_rev != res->max_rev)
Re: [PATCH] v4 Fix svnversion message as follow-up to r922176
Posted by Daniel Näslund <da...@longitudo.com>.
We're running out of patch versions here ...
gstein pointed out that I should use SVN_IS_VALID_REVNUM(). Fixed!
On Fri, Apr 02, 2010 at 10:59:15AM +0200, Daniel N�slund wrote:
> Mispelled Uncommitted with one 't'. This patch corrects that.
>
> On Fri, Apr 02, 2010 at 10:36:53AM +0200, Daniel N�slund wrote:
> > On Tue, Mar 30, 2010 at 11:04:13PM +0200, Stefan Sperling wrote:
> > > On Tue, Mar 30, 2010 at 10:14:42PM +0200, Daniel N�slund wrote:
> > > > Ping! This patch has not been reviewed!
> > > >
> > > > On Sun, Mar 14, 2010 at 09:38:15PM +0100, Daniel N�slund wrote:
> > > > > Hi!
> > > > >
> > > > > The 1.6 svnversion message was "'path' not versioned, and not exported".
> > > > > But on trunk more than one message has been changed. My first thought
> > > > > was that we should be backward compat in our output but if changes of
> > > > > those messages are ok I'm supplying one.
> > > > >
> > > > > In case we will use new messages, the help text must be updated. It
> > > > > talks of 'exported' but those are not used in the new messages.
> > > > >
> > > > > [[[
> > > > > After the changes in r922176, versioned but not yet committed files were
> > > > > not properly detected. Fixed now!
> > > > >
> > > > > * subversion/svnversion/main.c
> > > > > (main): Check for invalid rev nr for files and dirs.
> > > > >
> > > > > * subversion/tests/cmdline/svnversion_tests.py
> > > > > (structural_changes): New.
> > > > > (tests_list): Add new test.
> > > > > ]]]
Re: [PATCH] v4 Fix svnversion message as follow-up to r922176
Posted by Daniel Näslund <da...@longitudo.com>.
We're running out of patch versions here ...
gstein pointed out that I should use SVN_IS_VALID_REVNUM(). Fixed!
On Fri, Apr 02, 2010 at 10:59:15AM +0200, Daniel Näslund wrote:
> Mispelled Uncommitted with one 't'. This patch corrects that.
>
> On Fri, Apr 02, 2010 at 10:36:53AM +0200, Daniel Näslund wrote:
> > On Tue, Mar 30, 2010 at 11:04:13PM +0200, Stefan Sperling wrote:
> > > On Tue, Mar 30, 2010 at 10:14:42PM +0200, Daniel Näslund wrote:
> > > > Ping! This patch has not been reviewed!
> > > >
> > > > On Sun, Mar 14, 2010 at 09:38:15PM +0100, Daniel Näslund wrote:
> > > > > Hi!
> > > > >
> > > > > The 1.6 svnversion message was "'path' not versioned, and not exported".
> > > > > But on trunk more than one message has been changed. My first thought
> > > > > was that we should be backward compat in our output but if changes of
> > > > > those messages are ok I'm supplying one.
> > > > >
> > > > > In case we will use new messages, the help text must be updated. It
> > > > > talks of 'exported' but those are not used in the new messages.
> > > > >
> > > > > [[[
> > > > > After the changes in r922176, versioned but not yet committed files were
> > > > > not properly detected. Fixed now!
> > > > >
> > > > > * subversion/svnversion/main.c
> > > > > (main): Check for invalid rev nr for files and dirs.
> > > > >
> > > > > * subversion/tests/cmdline/svnversion_tests.py
> > > > > (structural_changes): New.
> > > > > (tests_list): Add new test.
> > > > > ]]]
Re: [PATCH] v2 Fix svnversion message as follow-up to r922176
Posted by Daniel Näslund <da...@longitudo.com>.
Mispelled Uncommitted with one 't'. This patch corrects that.
On Fri, Apr 02, 2010 at 10:36:53AM +0200, Daniel Näslund wrote:
> On Tue, Mar 30, 2010 at 11:04:13PM +0200, Stefan Sperling wrote:
> > On Tue, Mar 30, 2010 at 10:14:42PM +0200, Daniel Näslund wrote:
> > > Ping! This patch has not been reviewed!
> > >
> > > On Sun, Mar 14, 2010 at 09:38:15PM +0100, Daniel Näslund wrote:
> > > > Hi!
> > > >
> > > > The 1.6 svnversion message was "'path' not versioned, and not exported".
> > > > But on trunk more than one message has been changed. My first thought
> > > > was that we should be backward compat in our output but if changes of
> > > > those messages are ok I'm supplying one.
> > > >
> > > > In case we will use new messages, the help text must be updated. It
> > > > talks of 'exported' but those are not used in the new messages.
> > > >
> > > > [[[
> > > > After the changes in r922176, versioned but not yet committed files were
> > > > not properly detected. Fixed now!
> > > >
> > > > * subversion/svnversion/main.c
> > > > (main): Check for invalid rev nr for files and dirs.
> > > >
> > > > * subversion/tests/cmdline/svnversion_tests.py
> > > > (structural_changes): New.
> > > > (tests_list): Add new test.
> > > > ]]]
> > >
> > > > Index: subversion/svnversion/main.c
> > > > ===================================================================
> > > > --- subversion/svnversion/main.c (revision 922931)
> > > > +++ subversion/svnversion/main.c (arbetskopia)
> > > > @@ -290,6 +290,17 @@
> > > > return EXIT_FAILURE;
> > > > }
> > > >
> > > > + if (res->min_rev == -1)
> >
> > s/-1/SVN_INVALID_REVNUM/
>
> Fixed!
>
> > > > + {
> > > > + /* Local uncommited modifications, no revision info was found. */
> > > > + SVN_INT_ERR(svn_cmdline_printf(pool, _("Local uncommitted "
> > > > + "modifications, no revision "
> > > > + "information found%s"),
> > > > + no_newline ? "" : "\n"));
> >
> > "No revision information found" sounds a bit like something went
> > wrong, as in, Subversion was looking for information that was
> > expected to be present but it didn't find the information.
> > What about "Uncommitted local addition, copy, or move%s" instead?
>
> I agree with you (user messages is my weak spot). The only cases when we
> can have no revision number but the path is under version control is for
> locally added, copied or moved paths. So we tell the user about it.
>
> Can I have your +1 for commiting?
Re: [PATCH] v2 Fix svnversion message as follow-up to r922176
Posted by Daniel Näslund <da...@longitudo.com>.
Mispelled Uncommitted with one 't'. This patch corrects that.
On Fri, Apr 02, 2010 at 10:36:53AM +0200, Daniel N�slund wrote:
> On Tue, Mar 30, 2010 at 11:04:13PM +0200, Stefan Sperling wrote:
> > On Tue, Mar 30, 2010 at 10:14:42PM +0200, Daniel N�slund wrote:
> > > Ping! This patch has not been reviewed!
> > >
> > > On Sun, Mar 14, 2010 at 09:38:15PM +0100, Daniel N�slund wrote:
> > > > Hi!
> > > >
> > > > The 1.6 svnversion message was "'path' not versioned, and not exported".
> > > > But on trunk more than one message has been changed. My first thought
> > > > was that we should be backward compat in our output but if changes of
> > > > those messages are ok I'm supplying one.
> > > >
> > > > In case we will use new messages, the help text must be updated. It
> > > > talks of 'exported' but those are not used in the new messages.
> > > >
> > > > [[[
> > > > After the changes in r922176, versioned but not yet committed files were
> > > > not properly detected. Fixed now!
> > > >
> > > > * subversion/svnversion/main.c
> > > > (main): Check for invalid rev nr for files and dirs.
> > > >
> > > > * subversion/tests/cmdline/svnversion_tests.py
> > > > (structural_changes): New.
> > > > (tests_list): Add new test.
> > > > ]]]
> > >
> > > > Index: subversion/svnversion/main.c
> > > > ===================================================================
> > > > --- subversion/svnversion/main.c (revision 922931)
> > > > +++ subversion/svnversion/main.c (arbetskopia)
> > > > @@ -290,6 +290,17 @@
> > > > return EXIT_FAILURE;
> > > > }
> > > >
> > > > + if (res->min_rev == -1)
> >
> > s/-1/SVN_INVALID_REVNUM/
>
> Fixed!
>
> > > > + {
> > > > + /* Local uncommited modifications, no revision info was found. */
> > > > + SVN_INT_ERR(svn_cmdline_printf(pool, _("Local uncommitted "
> > > > + "modifications, no revision "
> > > > + "information found%s"),
> > > > + no_newline ? "" : "\n"));
> >
> > "No revision information found" sounds a bit like something went
> > wrong, as in, Subversion was looking for information that was
> > expected to be present but it didn't find the information.
> > What about "Uncommitted local addition, copy, or move%s" instead?
>
> I agree with you (user messages is my weak spot). The only cases when we
> can have no revision number but the path is under version control is for
> locally added, copied or moved paths. So we tell the user about it.
>
> Can I have your +1 for commiting?