Paths in the client patch code

["Hyrum K. Wright" <hy...@mail.utexas.edu>]

I see a lot of paths in the client patch code, but few of the variable names
are explicit about whether the path is absolute or relative.  Can somebody
who knows the code comment on what types of paths it uses?  If we do know,
we should probably rename the various patch variables as foo_abspath or
foo_relpath, as the circumstance may dictate.

-Hyrum

Re: Paths in the client patch code

[Stefan Sperling <st...@elego.de>]

On Tue, May 04, 2010 at 11:30:07AM +0200, Hyrum K. Wright wrote:
> On Tue, May 4, 2010 at 11:13 AM, Stefan Sperling <st...@elego.de> wrote:
> 
> > On Tue, May 04, 2010 at 10:54:51AM +0200, Hyrum K. Wright wrote:
> > > I see a lot of paths in the client patch code, but few of the variable
> > names
> > > are explicit about whether the path is absolute or relative.  Can
> > somebody
> > > who knows the code comment on what types of paths it uses?  If we do
> > know,
> > > we should probably rename the various patch variables as foo_abspath or
> > > foo_relpath, as the circumstance may dictate.
> >
> > Can you be more specific and list the variables which are unclear to you?
> > Then I'll try finding better names for them.
> 
> 
> In particular, the patched_path, reject_path members of the patch_target_t
> struct.  But more generally, most variables that end in _path, shouldn't.

Those are returned by svn_stream_open_unique(), the docco of which
says "and the full path will be returned in @a temp_path."
Case we can rename those to patched_abspath and reject_abspath.

There is a 'path' variable in the strip_path() function.
It can be either absolute or relative, depending on what paths
the patch file contains. Same for the 'stripped_path' variable in
resolve_target_path(). And 'canon_path_from_patchfile' also depends
on the patch file input.

There's a 'parent_path' in the status baton, which should probably
be parent_abspath, depending on how svn_wc_walk_status() behaves.

The other paths I see in libsvn_client/patch.c already carry 'rel'
or 'abs' in their name.

Stefan

Re: Paths in the client patch code

["Hyrum K. Wright" <hy...@mail.utexas.edu>]

On Tue, May 4, 2010 at 11:13 AM, Stefan Sperling <st...@elego.de> wrote:

> On Tue, May 04, 2010 at 10:54:51AM +0200, Hyrum K. Wright wrote:
> > I see a lot of paths in the client patch code, but few of the variable
> names
> > are explicit about whether the path is absolute or relative.  Can
> somebody
> > who knows the code comment on what types of paths it uses?  If we do
> know,
> > we should probably rename the various patch variables as foo_abspath or
> > foo_relpath, as the circumstance may dictate.
>
> Can you be more specific and list the variables which are unclear to you?
> Then I'll try finding better names for them.


In particular, the patched_path, reject_path members of the patch_target_t
struct.  But more generally, most variables that end in _path, shouldn't.

-Hyrum

Re: Paths in the client patch code

[Stefan Sperling <st...@elego.de>]

On Tue, May 04, 2010 at 10:54:51AM +0200, Hyrum K. Wright wrote:
> I see a lot of paths in the client patch code, but few of the variable names
> are explicit about whether the path is absolute or relative.  Can somebody
> who knows the code comment on what types of paths it uses?  If we do know,
> we should probably rename the various patch variables as foo_abspath or
> foo_relpath, as the circumstance may dictate.

Can you be more specific and list the variables which are unclear to you?
Then I'll try finding better names for them.

Thanks,
Stefan