You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Madan U S <ma...@collab.net> on 2006/03/31 15:07:43 UTC

[PATCH] svnmerge.py: all error handling to be done using the error() function

[[[
All error handling to be done using the error() function.

* contrib/client-side/svnmerge.py
  (error): Moved function to top of the file (well, almost ;)
  (global - during check for python version):
  (main):
  (check_old_prop_version): Replace code to print error message, exit(1)
                            with call to error().
]]]

RE: Re: [Svnmerge] [PATCH] svnmerge.py: all error handling to be done using the error() function

Posted by Madan U S <ma...@collab.net>.
On Saturday 01 Apr 2006 12:04 am, Daniel Rall wrote:
> On Fri, 31 Mar 2006, Madan S. wrote:
> > All error handling to be done using the error() function.
>
> ...
>
> I've committed a tweaked patch to Subversion's trunk in r19112.

Thank you.

Regards,
Madan.

Re: [Svnmerge] [PATCH] svnmerge.py: all error handling to be done using the error() function

Posted by Daniel Rall <dl...@collab.net>.
On Fri, 31 Mar 2006, Madan S. wrote:

> All error handling to be done using the error() function.
...

I've committed a tweaked patch to Subversion's trunk in r19112.

- Dan

Re: [Svnmerge] [PATCH] svnmerge.py: all error handling to be done usingthe error() function

Posted by Giovanni Bajo <ra...@develer.com>.
Madan U S <ma...@collab.net> wrote:

> [[[
> All error handling to be done using the error() function.
> 
> * contrib/client-side/svnmerge.py
>   (error): Moved function to top of the file (well, almost ;)
>   (global - during check for python version):
>   (main):
>   (check_old_prop_version): Replace code to print error message, exit(1)
>                             with call to error().
> ]]]


Thanks for the cleanup!

>>     except KeyboardInterrupt:
>>         # Avoid traceback on CTRL+C
>>-        print "aborted by user"
>>-        sys.exit(1)
>>+        err_str = "aborted by user"
>>+        error(err_str)

You don't need err_str here.
-- 
Giovanni Bajo

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [Svnmerge] [PATCH] svnmerge.py: all error handling to be doneusing the error() function

Posted by David James <dj...@collab.net>.
On 3/31/06, Madan U S <ma...@collab.net> wrote:
>
>
> Daniel Rall said:
>
>  > madan said:
>  > +def error(s):
>  > +    """Subroutine to output an error and bail."""
>  > +    print >> sys.stderr, "%s: %s" % (NAME, s)
>  > +    sys.exit(1)
>  > +
>
>  > > We don't really need to relocate the definition of this function
>  > > within the source file.
>
>  I would think so too. But I tried with a sample pythong script. It expected
> the declaration of a function to preceed the usage. Wondering what could be
> different?!

In Python, functions must be declared before they are called. If your
"main" function is only called at the bottom of the program, then you
can call any function in the program from the main function. For
example:
   def main():
      hello()
   def hello():
     print "hello"
   main()

If you move the call to "main()" up above the declaration of "hello",
then the program does not work. This is the difference.

Cheers,

David




--
David James -- http://www.cs.toronto.edu/~james

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: [Svnmerge] [PATCH] svnmerge.py: all error handling to be doneusing the error() function

Posted by Daniel Rall <dl...@collab.net>.
On Fri, 31 Mar 2006, Madan S. wrote:

> Daniel Rall said:
> > madan said:
> > +def error(s):
> > +    """Subroutine to output an error and bail."""
> > +    print >> sys.stderr, "%s: %s" % (NAME, s)
> > +    sys.exit(1)
> > +
> 
> > > We don't really need to relocate the definition of this function
> > > within the source file.
> 
> I would think so too. But I tried with a sample pythong script. It
> expected the declaration of a function to preceed the
> usage. Wondering what could be different?!


As this is Python (rather than C), that's not required.
<soapbox>Stylistically speaking, when the implementation language
allows it, I prefer definition of high-level code to proceed
lower-level utility routines within a source file (e.g. public stuff
before private stuff), as I feel it results in more easily
understandable code for those who come later (it's top-down,
literally).</soapbox> Also, it avoids spurious diffs in the change
history.  As usual, YMMV.
-- 

Daniel Rall

RE: [Svnmerge] [PATCH] svnmerge.py: all error handling to be doneusing the error() function

Posted by Madan U S <ma...@collab.net>.
Daniel Rall said:
> madan said:
> +def error(s):
> +    """Subroutine to output an error and bail."""
> +    print >> sys.stderr, "%s: %s" % (NAME, s)
> +    sys.exit(1)
> +

> > We don't really need to relocate the definition of this function
> > within the source file.

I would think so too. But I tried with a sample pythong script. It expected the declaration of a function to preceed the usage. Wondering what could be different?!

Regards,
Madan.

Re: [Svnmerge] [PATCH] svnmerge.py: all error handling to be done using the error() function

Posted by Daniel Rall <dl...@collab.net>.
...
> --- contrib/client-side/svnmerge.py	(revision 19109)
> +++ contrib/client-side/svnmerge.py	(working copy)
> @@ -56,10 +56,14 @@
>  import sys, os, getopt, re, types, popen2, tempfile
>  from bisect import bisect
>  
> +def error(s):
> +    """Subroutine to output an error and bail."""
> +    print >> sys.stderr, "%s: %s" % (NAME, s)
> +    sys.exit(1)
> +

We don't really need to relocate the definition of this function
within the source file.

...
> @@ -1629,11 +1627,12 @@
>      try:
>          main(sys.argv[1:])
>      except LaunchError, (ret, cmd, out):
> -        print "%s: command execution failed (exit code: %d)" % (NAME, ret)
> -        print cmd
> -        print "".join(out)
> -        sys.exit(1)
> +        err_str  = '%s: command execution failed ' % NAME

There's no reason to prepend NAME if we're using error(), since that
function handles the prepending for us.

> +        err_str += '(exit code: %d)\n' % ret
> +        err_str += cmd + '\n'
> +        err_str += "".join(out)
> +        error(err_str)
>      except KeyboardInterrupt:
>          # Avoid traceback on CTRL+C
> -        print "aborted by user"
> -        sys.exit(1)
> +        err_str = "aborted by user"
> +        error(err_str)


Is a control-c really an error?  I'd certainly rather see this output
on stdout than stderr.

-- 

Daniel Rall