RE: Subversion Python bindings

[Barry Scott <ba...@barrys-emacs.org>]

I'll add users@subversion.tigris.org so other see the discussion. There
are a number of comments there already.

Barry

At 03-11-2003 20:18, John Barstow wrote:
>Attached is a first pass at doc strings for just over half the methods.  I
>hope to work on the other half tonight; I also have a HOWTO with common
>workflow tasks in the works.
>
>In reviewing, I noticed that the ability to relocate the repository seems to
>be missing (svn switch --relocate); this is important when the repository
>URL changes.

O.K. I'll look into it.

Also how is the revision range passed to svn_update? That one for the
rapidsvn folks.

> > What are think a "familiar status code" looks like in python?
> > repr would tell you what it was "<svn_revision_kind_t head>" for example.
>I was thinking of the A, M, ? codes the command line returns.  An
>enumeration works better if we add repr.

What is "A, M,"?

O.K. I'll try the enum in a python type and we can see how that works out.

> > What are you thinking of? I think being pythonic is very important.
>Couple of things:
>The svn_ prefix is redundant.  I believe client.add() is more pythonic than
>client.svn_add().

I want all commands to have the same naming pattern. If I do not have a
prefix then I end up with one command that is a keyword, "import". There
is a problem in C++ with the "switch" command. The C++ code uses cmd_ as
the prefix.

>The enumerations could be better named(svn_revision_kind_t is hardly
>intuitive).
>We should be more consistent about path vs. path_list.  Ideally both would
>work; I should be able to write both of the following:
>
>client.add("foo.c")
>newfiles = ["bar.c", "baz/"]
>client.add(newfiles)

O.K. The code is easy, I wonder if there is a down side?

>Having recurse default to False in places where the command line usually
>defaults to True will probably cause problems.  It's documented but not
>obvious.  It's surprising to do a checkout and only get the top-level
>directory.

O.K. I'll default all to recurse True. Your the second person to say the
defaults are wrong.

>Lastly, I would expect to pass either a revision number or a revision kind
>to the client api. So:
>
>client.update("foo.c", 1579)
>client.update("foo.c", revision.HEAD)

The goal of the API is to be obvious to use, extensible and error free to use.
I'm not convinced that adding this sort of short hand is good.

I like the way the keywords make it clear which booleans you are setting.

>As an example, here's the snippet I would like to write:
>import pysvn
>client = pysvn.client()
>wc = "/project/subversion"
>client.update(wc)
># make some changes
>client.add("foo.c")
>client.remove("bar.c")
>#add all unversioned files
>changedfiles = client.status(wc)
>client.add([s.file for s in changedfiles if s.status ==
>pysvn.status.UNVERSIONED])
>client.revert("tmp/", True)
>client.commit(wc)

This requires:
* change the client function names. See above for import problem
* remove keyword recurse from revert. That would be inconsistent.
* support path and lists_of_paths everywhere. Should be reasonable to do.
* find short name for svn enums. removing the svn_ prefix is easy
   but I'm not sure I should remove much more. Over time we may need to
   expose other enums that will conflict with contraction to just status.
   the _t suffix can go as well.
* upper case all enum values. Why did you want the names upper cased?

I'll work on a 0.2 will some of these changes.

Barry



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

RE: Subversion Python bindings

[Barry Scott <ba...@barrys-emacs.org>]

At 07-11-2003 07:38, John C Barstow wrote:
>Sorry about the delay; very busy week at work. I'll be getting you
>updates over the weekend.

You may want to wait till I get 0.2 out, hopefully tonight.

>On Wed, 2003-11-05 at 05:14, Barry Scott wrote:
>
> > What is "A, M,"?
> >
>The status codes shown when you do "svn st" at the command line ('A' for
>added, 'D' for removed, etc...)

The A and M is not the domain of the extension. That's a UI issue. The
svn_cmd.py implements this as a mapping of the status codes.

BTW I have done a lot of work on svn_cmd.py. It is evolving from a example
into a full replacement for svn.exe. I have command line options parsing
implemented and a lot of the commands. It's also proving a good way to
find out what is missing from the svncpp API that we will need. I think
svn_cmd makes an interest alternative Python API to SVN. Since you can
leverage knowledge of the command line utility to issues commands out
of native python:

import svn_cmd
svn_cmd.main( [sys.argv[0], 'co', 'http://blah/repo/trunc/x', 'workspace-x'] )

> > O.K. I'll try the enum in a python type and we can see how that works out.
>...
>I was thinking about this.  What if we left the C++ as it is, and simply
>wrapped another class around it with more pythonic names?

I have added an enum_value object that is working well and shorted all the
names. I don't think we should add a wrapped when I can make the extension
do what the wrapper adds.

>I seem to recall that there were a couple of commands that were
>non-recursive by default at the command line.  If that's still the case,
>we'll also want those particular commands to default the same way.

Yes, I'm reviewing each command to see if its needs a true or false default.

> > >Lastly, I would expect to pass either a revision number or a revision kind
> > >to the client api. So:
> > >
> > >client.update("foo.c", 1579)
> > >client.update("foo.c", revision.HEAD)
> >
> > The goal of the API is to be obvious to use, extensible and error free 
> to use.
> > I'm not convinced that adding this sort of short hand is good.
>Well, you're always passing either a number or one of the keywords as a
>revision. I think this is the obvious way to do it, but it's definitely
>not a big deal to do it the current way.

The problem is that its either a enum, number as revnum or number as date,
the last two are ambiguous. Where as requiring an enum with optional
revnum or date is not ambiguous.

> > I like the way the keywords make it clear which booleans you are setting.
> >
> > >As an example, here's the snippet I would like to write:
> > >import pysvn
> > >client = pysvn.client()
> > >wc = "/project/subversion"
> > >client.update(wc)
> > ># make some changes
> > >client.add("foo.c")
> > >client.remove("bar.c")
> > >#add all unversioned files
> > >changedfiles = client.status(wc)
> > >client.add([s.file for s in changedfiles if s.status ==
> > >pysvn.status.UNVERSIONED])
> > >client.revert("tmp/", True)
> > >client.commit(wc)
> >
> > This requires:
> > * change the client function names. See above for import problem
>We will have come up with another name for import.

I'm using import_ for the next version, someone else suggested that name
while arguing at length to have me lose the svn_ prefixes.

> > * remove keyword recurse from revert. That would be inconsistent.
>I meant the True to be the recurse argument.
>
> > * support path and lists_of_paths everywhere. Should be reasonable to do.
> > * find short name for svn enums. removing the svn_ prefix is easy
> >    but I'm not sure I should remove much more. Over time we may need to
> >    expose other enums that will conflict with contraction to just status.
> >    the _t suffix can go as well.
>A wrapper class could allow us to leave the C++ as is.

There are commands like mkdir that take a list in svncpp because all
the dirs are added to the repo in one transaction with one log message.
Others that do not modify the repo do not need to take a list. I can,
with no problem, allow most not repo commands to take a list of a string
as the path. Commands like diff would be harder to make intuitive.
I need to make sure that all the repo affecting commands allow lists.

> > * upper case all enum values. Why did you want the names upper cased?
>That's my coding style; holdover from my C++ days, I guess, when all
>constants were uppercased.  We should do whatever the Python style guide
>says.

I'll stick with lower then.

>John C Barstow

Barry



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

RE: Subversion Python bindings

[John C Barstow <jb...@amathaine.com>]

Sorry about the delay; very busy week at work. I'll be getting you
updates over the weekend.

On Wed, 2003-11-05 at 05:14, Barry Scott wrote:

> What is "A, M,"?
> 
The status codes shown when you do "svn st" at the command line ('A' for
added, 'D' for removed, etc...)

> O.K. I'll try the enum in a python type and we can see how that works out.
> 
> > > What are you thinking of? I think being pythonic is very important.
> >Couple of things:
> >The svn_ prefix is redundant.  I believe client.add() is more pythonic than
> >client.svn_add().
> 
> I want all commands to have the same naming pattern. If I do not have a
> prefix then I end up with one command that is a keyword, "import". There
> is a problem in C++ with the "switch" command. The C++ code uses cmd_ as
> the prefix.
> 
I was thinking about this.  What if we left the C++ as it is, and simply
wrapped another class around it with more pythonic names?
For example:
(pysvn.py)
import libpysvn #C++ extension
class client:
    def __init__(self):
        self.libsvn = libpysvn.client()
    def add(path):
        self.libsvn.svn_add(path)
The user gets pythonic names, you keep the C++ code clean and obvious.
> >The enumerations could be better named(svn_revision_kind_t is hardly
> >intuitive).
> >We should be more consistent about path vs. path_list.  Ideally both would
> >work; I should be able to write both of the following:
> >
> >client.add("foo.c")
> >newfiles = ["bar.c", "baz/"]
> >client.add(newfiles)
> 
> O.K. The code is easy, I wonder if there is a down side?
> 
> >Having recurse default to False in places where the command line usually
> >defaults to True will probably cause problems.  It's documented but not
> >obvious.  It's surprising to do a checkout and only get the top-level
> >directory.
> 
> O.K. I'll default all to recurse True. Your the second person to say the
> defaults are wrong.
> 
I seem to recall that there were a couple of commands that were
non-recursive by default at the command line.  If that's still the case,
we'll also want those particular commands to default the same way.

> >Lastly, I would expect to pass either a revision number or a revision kind
> >to the client api. So:
> >
> >client.update("foo.c", 1579)
> >client.update("foo.c", revision.HEAD)
> 
> The goal of the API is to be obvious to use, extensible and error free to use.
> I'm not convinced that adding this sort of short hand is good.
Well, you're always passing either a number or one of the keywords as a
revision. I think this is the obvious way to do it, but it's definitely
not a big deal to do it the current way.

> I like the way the keywords make it clear which booleans you are setting.
> 
> >As an example, here's the snippet I would like to write:
> >import pysvn
> >client = pysvn.client()
> >wc = "/project/subversion"
> >client.update(wc)
> ># make some changes
> >client.add("foo.c")
> >client.remove("bar.c")
> >#add all unversioned files
> >changedfiles = client.status(wc)
> >client.add([s.file for s in changedfiles if s.status ==
> >pysvn.status.UNVERSIONED])
> >client.revert("tmp/", True)
> >client.commit(wc)
> 
> This requires:
> * change the client function names. See above for import problem
We will have come up with another name for import.

> * remove keyword recurse from revert. That would be inconsistent.
I meant the True to be the recurse argument.

> * support path and lists_of_paths everywhere. Should be reasonable to do.
> * find short name for svn enums. removing the svn_ prefix is easy
>    but I'm not sure I should remove much more. Over time we may need to
>    expose other enums that will conflict with contraction to just status.
>    the _t suffix can go as well.
A wrapper class could allow us to leave the C++ as is.

> * upper case all enum values. Why did you want the names upper cased?
That's my coding style; holdover from my C++ days, I guess, when all
constants were uppercased.  We should do whatever the Python style guide
says.

John C Barstow


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

Re: Subversion Python bindings

["Jens B. Jorgensen" <je...@tallan.com>]

Barry Scott wrote:

> I'll add users@subversion.tigris.org so other see the discussion. There
> are a number of comments there already.
>
> Barry
>
> At 03-11-2003 20:18, John Barstow wrote:
>
>> Attached is a first pass at doc strings for just over half the 
>> methods.  I
>> hope to work on the other half tonight; I also have a HOWTO with common
>> workflow tasks in the works.
>>
>> In reviewing, I noticed that the ability to relocate the repository 
>> seems to
>> be missing (svn switch --relocate); this is important when the 
>> repository
>> URL changes.
>
>
> O.K. I'll look into it.
>
> Also how is the revision range passed to svn_update? That one for the
> rapidsvn folks.
>
>> > What are think a "familiar status code" looks like in python?
>> > repr would tell you what it was "<svn_revision_kind_t head>" for 
>> example.
>> I was thinking of the A, M, ? codes the command line returns.  An
>> enumeration works better if we add repr.
>
>
> What is "A, M,"?
>
> O.K. I'll try the enum in a python type and we can see how that works 
> out.
>
>> > What are you thinking of? I think being pythonic is very important.
>> Couple of things:
>> The svn_ prefix is redundant.  I believe client.add() is more 
>> pythonic than
>> client.svn_add().
>
>
> I want all commands to have the same naming pattern. If I do not have a
> prefix then I end up with one command that is a keyword, "import". There
> is a problem in C++ with the "switch" command. The C++ code uses cmd_ as
> the prefix.

There are certainly other python modules where keyword avoidance has to 
be overcome. Which is a bigger wort, having everything (well, almost 
everything) prefixed with 'svn_' or having the import command be 
'import_', 'imp', ...

I can understand the virtue of having consistency between naming on the 
python side and c++ side (I like to write c++ extension modules myself) 
but is this really such a big maintenance trap? Anyone who writes python 
already knows that 'import' is a keyword and this the corresponding 
python method must be called something like that. Ditto with switch on 
the c++ side. I don't see this as the kind of thing that ends up causing 
some hapless programmer to sit there scratching their heads over an 
exception for 2 days or from introducing some lurking bug beast that 
would lay dormant until one day.

You could look at it this way: you've already gone down the road to 
deliver a 'pythonic' interface as opposed to the already existing c-ish 
one. Having done that why have the c implementation continue to bleed 
through like this? Contrary to what you said earlier I don't think the 
'average' consumer of this module (at least down the road) will ever 
look at the C source to understand the interface. That user is going to 
be interesting purely in getting the good pythonic interface to svn. 
That person will immediately jump up and post to a mailing list about 
how dumb it is to have all the methods prefixed with 'svn_' creating all 
sorts of useless traffic. ;-)

-- 
Jens B. Jorgensen
jens.jorgensen@tallan.com

"With a focused commitment to our clients and our people, we deliver value through customized technology solutions."



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