You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2002/05/07 17:59:27 UTC
Re: svn commit: rev 1895 - trunk/subversion/clients/cmdline trunk/subversion/tests/clients/cmdline/svntest
On Tue, May 07, 2002 at 11:18:40AM -0500, kfogel@tigris.org wrote:
>...
> +++ trunk/subversion/clients/cmdline/trace-update.c Tue May 7 11:18:26 2002
>...
> @@ -331,6 +336,10 @@
> struct file_baton *fb = file_baton;
> if (svn_wc_is_normal_prop (name))
> fb->prop_changed = TRUE;
> +
> + if (svn_wc_is_normal_prop (name))
> + fb->edit_baton->received_some_change = TRUE;
It would be clearer if those two if-blocks were combined. As it is, a reader
has to look harder to realize that they are the same. Combining reduces
their effort.
>...
> @@ -344,6 +353,10 @@
> struct dir_baton *db = parent_baton;
> if (svn_wc_is_normal_prop (name))
> db->prop_changed = TRUE;
> +
> + if (svn_wc_is_normal_prop (name))
> + db->edit_baton->received_some_change = TRUE;
> +
> return SVN_NO_ERROR;
Same.
But even better: in the close_file and close_directory functions, just
insert a line like:
fb->edit_baton->received_some_change ||= fb->prop_changed;
And similar for close_directory.
>...
> @@ -390,6 +409,7 @@
> eb->target_revision = SVN_INVALID_REVNUM;
> eb->is_checkout = is_checkout;
> eb->suppress_final_line = suppress_final_line;
> + eb->received_some_change = FALSE;
FALSE is the default already, because of the apr_pcalloc()
>...
> +++ trunk/subversion/tests/clients/cmdline/svntest/tree.py Tue May 7 11:18:26 2002
> @@ -535,7 +535,7 @@
> "Return a tree derived by parsing the output LINES from 'co' or 'up'."
>
> root = SVNTreeNode(root_node_name)
> - rm = re.compile ('^(..)\s+(.+)')
> + rm = re.compile ('^([MAGCUD_ ][MAGCUD_ ])\s+(.+)')
>
> for line in lines:
> match = rm.search(line)
> @@ -591,7 +591,8 @@
> else:
> repos_rev = '?'
>
> - rm = re.compile ('^(..)(.)(.) . [^0-9]+(\d+|-)(.{23})(.+)')
> + # Try http://www.wordsmith.org/anagram/anagram.cgi?anagram=ACDRMGU
> + rm = re.compile ('^([MACDRUG_ ][MACDRUG_ ])(.)(.) . [^0-9]+(\d+|-)(.{23})(.+)')
Ha. I told you guys we wanted different column-0 characters.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 1895 - trunk/subversion/clients/cmdline trunk/subversion/tests/clients/cmdline/svntest
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Greg Stein <gs...@lyra.org> writes:
> It would be clearer if those two if-blocks were combined. As it is, a reader
> has to look harder to realize that they are the same. Combining reduces
> their effort.
Indeed -- if this reader had looked harder, he would have seen that!
Thanks!
> >...
> > @@ -344,6 +353,10 @@
> > struct dir_baton *db = parent_baton;
> > if (svn_wc_is_normal_prop (name))
> > db->prop_changed = TRUE;
> > +
> > + if (svn_wc_is_normal_prop (name))
> > + db->edit_baton->received_some_change = TRUE;
> > +
> > return SVN_NO_ERROR;
>
> Same.
Check, same for the next
> > @@ -390,6 +409,7 @@
> > eb->target_revision = SVN_INVALID_REVNUM;
> > eb->is_checkout = is_checkout;
> > eb->suppress_final_line = suppress_final_line;
> > + eb->received_some_change = FALSE;
>
> FALSE is the default already, because of the apr_pcalloc()
Good point.
> > - rm = re.compile ('^(..)(.)(.) . [^0-9]+(\d+|-)(.{23})(.+)')
> > + # Try http://www.wordsmith.org/anagram/anagram.cgi?anagram=ACDRMGU
> > + rm = re.compile ('^([MACDRUG_ ][MACDRUG_ ])(.)(.) . [^0-9]+(\d+|-)(.{23})(.+)')
>
> Ha. I told you guys we wanted different column-0 characters.
:-)
No, seriously, the current way is better, I think.
Our goal is to have output that is human-readable and automatically
parseable. The latter is a formal, yes-or-no condition, and we still
meet it. There is no requirement that parsing be completely trivial
to implement, especially not if it means sacrificing human
readability.
If we start reserving column zero characters in lieu of good regexps,
we effectively make it impossible to start any prose sentence in
column zero, even though that's the natural place for sentences to
start.
There will always be compromises like this, as long we're trying to
meet two slightly incompatible goals for the output. We should
usually resolve these conflicts in favor of the humans, IMHO, except
when it would put a truly unreasonable burden on the parser (which
isn't even close to happening here).
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 1895 - trunk/subversion/clients/cmdline trunk/subversion/tests/clients/cmdline/svntest
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Greg Stein <gs...@lyra.org> writes:
> But even better: in the close_file and close_directory functions, just
> insert a line like:
>
> fb->edit_baton->received_some_change ||= fb->prop_changed;
>
> And similar for close_directory.
For consistency I'd prefer to keep it as close to the actual change as
possible (since we can't depend on props or text changing,
particularly with add_file and add_dir, we can't confine it to
close_foo anyway).
Rest of changes made, thanks for the review!
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org