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 Rall <dl...@finemaltcoding.com> on 2005/10/20 16:37:34 UTC
Re: svn commit: r16853 - trunk/subversion/libsvn_wc
On Thu, 20 Oct 2005, zhakov@tigris.org wrote:
...
> New Revision: 16853
...
> Abstract common code from svn_wc__merge_props() and
> svn_wc__merge_prop_diffs()
>
> * subversion/libsvn_wc/props.c
> (open_reject_tmp_file): New helper for creating temporary reject file.
> Abstracted from svn_wc__merge_props().
> (svn_wc__merge_props, svn_wc__merge_prop_diffs): Eliminate slash variable.
> Use open_reject_tmp_file() helper.
...
> --- trunk/subversion/libsvn_wc/props.c (original)
> +++ trunk/subversion/libsvn_wc/props.c Thu Oct 20 10:34:03 2005
> @@ -208,6 +208,44 @@
>
> /*** Misc ***/
>
> +/* Opens reject temporary file for FULL_PATH. */
> +static svn_error_t *
> +open_reject_tmp_file (apr_file_t **fp, const char **reject_tmp_path,
> + const char *full_path,
> + svn_wc_adm_access_t *adm_access,
> + svn_boolean_t is_dir, apr_pool_t *pool)
> +{
> + const char *tmppath, *tmpname;
There is a parameter named "reject_tmp_path" to this function, yet neither
of the local variables defined above uses that same -- slightly more
comprehensible -- naming convention which uses the underscore to emphasize
word boundries (e.g. "tmp_path"). Both style came from pre-refactoring, but
it would be nice if the refactoring made them consistent.
> + /* Get path to /temporary/ local prop file */
> + SVN_ERR (svn_wc__prop_path (&tmppath, full_path, adm_access, TRUE, pool));
> +
> + /* Reserve a .prej file based on it. */
> + SVN_ERR (svn_io_open_unique_file (fp, reject_tmp_path, tmppath,
> + SVN_WC__PROP_REJ_EXT, FALSE, pool));
> +
> + /* reject_tmp_path is an absolute path at this point,
> + but that's no good for us. We need to convert this
> + path to a *relative* path to use in the logfile. */
> + tmpname = svn_path_basename (*reject_tmp_path, pool);
> +
> + if (is_dir)
> + {
> + /* Dealing with directory "path" */
> + *reject_tmp_path = svn_wc__adm_path ("", TRUE, /* use tmp */ pool,
> + tmpname, NULL);
Formatting a little off here?
...
> - }
> - }
> + /* This is the very first prop conflict found on this item. */
> + SVN_ERR (open_reject_tmp_file (&reject_tmp_fp, &reject_tmp_path,
> + full_path, adm_access, is_dir,
> + pool));
Formatting looks a touch odd here.
> /* Append the conflict to the open tmp/PROPS/---.prej file */
> SVN_ERR (append_prop_conflict (reject_tmp_fp,
...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r16853 - trunk/subversion/libsvn_wc
Posted by Ivan Zhakov <ch...@gmail.com>.
On 10/21/05, Garrett Rooney <ro...@electricjellyfish.net> wrote:
> On 10/21/05, Ivan Zhakov <ch...@gmail.com> wrote:
>
> > Fixed this and indentation in r16890. Thank you for review.
>
> Actually, there are two more places in the file where the indenting is
> screwed up, basically since you're using visual studio, and it
> defaults to tab == 4 spaces, anyplace tabs were introduced looked fine
> to you, but looks bad to everyone who uses an editor that thinks tabs
> are 8 spaces, which is of course why we don't like tabs ;-)
>
> I've removed them in revision 16893.
I am hate tabs too. Problem was because I switched to VC6 from VS.NET
and forget check tabs settings. Sorry. I'll be more carefull.
--
Ivan Zhakov
Re: svn commit: r16853 - trunk/subversion/libsvn_wc
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 10/21/05, Ivan Zhakov <ch...@gmail.com> wrote:
> Fixed this and indentation in r16890. Thank you for review.
Actually, there are two more places in the file where the indenting is
screwed up, basically since you're using visual studio, and it
defaults to tab == 4 spaces, anyplace tabs were introduced looked fine
to you, but looks bad to everyone who uses an editor that thinks tabs
are 8 spaces, which is of course why we don't like tabs ;-)
I've removed them in revision 16893.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r16853 - trunk/subversion/libsvn_wc
Posted by Ivan Zhakov <ch...@gmail.com>.
On 10/20/05, Daniel Rall <dl...@finemaltcoding.com> wrote:
> On Thu, 20 Oct 2005, Ivan Zhakov wrote:
>
> > On 10/20/05, Daniel Rall <dl...@finemaltcoding.com> wrote:
> > > On Thu, 20 Oct 2005, zhakov@tigris.org wrote:
> ...
> > > > --- trunk/subversion/libsvn_wc/props.c (original)
> > > > +++ trunk/subversion/libsvn_wc/props.c Thu Oct 20 10:34:03 2005
> > > > @@ -208,6 +208,44 @@
> > > >
> > > > /*** Misc ***/
> > > >
> > > > +/* Opens reject temporary file for FULL_PATH. */
> > > > +static svn_error_t *
> > > > +open_reject_tmp_file (apr_file_t **fp, const char **reject_tmp_path,
> > > > + const char *full_path,
> > > > + svn_wc_adm_access_t *adm_access,
> > > > + svn_boolean_t is_dir, apr_pool_t *pool)
> > > > +{
> > > > + const char *tmppath, *tmpname;
> > >
> > > There is a parameter named "reject_tmp_path" to this function, yet neither
> > > of the local variables defined above uses that same -- slightly more
> > > comprehensible -- naming convention which uses the underscore to emphasize
> > > word boundries (e.g. "tmp_path"). Both style came from pre-refactoring, but
> > > it would be nice if the refactoring made them consistent.
> >
> > To be clear: do you propose simply rename tmppath and tmpname
> > variables to tmp_path and tmp_name and nothing more? I don't take
> > myslef rename it on refactoring, because I am nott familiar with
> > subversion naming conventions in edge cases.
>
> Yeah, that's what I am suggesting.
>
Fixed this and indentation in r16890. Thank you for review.
--
Ivan Zhakov
Re: svn commit: r16853 - trunk/subversion/libsvn_wc
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Thu, 20 Oct 2005, Ivan Zhakov wrote:
> On 10/20/05, Daniel Rall <dl...@finemaltcoding.com> wrote:
> > On Thu, 20 Oct 2005, zhakov@tigris.org wrote:
...
> > > --- trunk/subversion/libsvn_wc/props.c (original)
> > > +++ trunk/subversion/libsvn_wc/props.c Thu Oct 20 10:34:03 2005
> > > @@ -208,6 +208,44 @@
> > >
> > > /*** Misc ***/
> > >
> > > +/* Opens reject temporary file for FULL_PATH. */
> > > +static svn_error_t *
> > > +open_reject_tmp_file (apr_file_t **fp, const char **reject_tmp_path,
> > > + const char *full_path,
> > > + svn_wc_adm_access_t *adm_access,
> > > + svn_boolean_t is_dir, apr_pool_t *pool)
> > > +{
> > > + const char *tmppath, *tmpname;
> >
> > There is a parameter named "reject_tmp_path" to this function, yet neither
> > of the local variables defined above uses that same -- slightly more
> > comprehensible -- naming convention which uses the underscore to emphasize
> > word boundries (e.g. "tmp_path"). Both style came from pre-refactoring, but
> > it would be nice if the refactoring made them consistent.
>
> To be clear: do you propose simply rename tmppath and tmpname
> variables to tmp_path and tmp_name and nothing more? I don't take
> myslef rename it on refactoring, because I am nott familiar with
> subversion naming conventions in edge cases.
Yeah, that's what I am suggesting.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r16853 - trunk/subversion/libsvn_wc
Posted by Ivan Zhakov <ch...@gmail.com>.
On 10/20/05, Daniel Rall <dl...@finemaltcoding.com> wrote:
> On Thu, 20 Oct 2005, zhakov@tigris.org wrote:
> ...
> > New Revision: 16853
> ...
> > Abstract common code from svn_wc__merge_props() and
> > svn_wc__merge_prop_diffs()
> >
> > * subversion/libsvn_wc/props.c
> > (open_reject_tmp_file): New helper for creating temporary reject file.
> > Abstracted from svn_wc__merge_props().
> > (svn_wc__merge_props, svn_wc__merge_prop_diffs): Eliminate slash variable.
> > Use open_reject_tmp_file() helper.
> ...
> > --- trunk/subversion/libsvn_wc/props.c (original)
> > +++ trunk/subversion/libsvn_wc/props.c Thu Oct 20 10:34:03 2005
> > @@ -208,6 +208,44 @@
> >
> > /*** Misc ***/
> >
> > +/* Opens reject temporary file for FULL_PATH. */
> > +static svn_error_t *
> > +open_reject_tmp_file (apr_file_t **fp, const char **reject_tmp_path,
> > + const char *full_path,
> > + svn_wc_adm_access_t *adm_access,
> > + svn_boolean_t is_dir, apr_pool_t *pool)
> > +{
> > + const char *tmppath, *tmpname;
>
> There is a parameter named "reject_tmp_path" to this function, yet neither
> of the local variables defined above uses that same -- slightly more
> comprehensible -- naming convention which uses the underscore to emphasize
> word boundries (e.g. "tmp_path"). Both style came from pre-refactoring, but
> it would be nice if the refactoring made them consistent.
To be clear: do you propose simply rename tmppath and tmpname
variables to tmp_path and tmp_name and nothing more? I don't take
myslef rename it on refactoring, because I am nott familiar with
subversion naming conventions in edge cases.
> > + /* Get path to /temporary/ local prop file */
> > + SVN_ERR (svn_wc__prop_path (&tmppath, full_path, adm_access, TRUE, pool));
> > +
> > + /* Reserve a .prej file based on it. */
> > + SVN_ERR (svn_io_open_unique_file (fp, reject_tmp_path, tmppath,
> > + SVN_WC__PROP_REJ_EXT, FALSE, pool));
> > +
> > + /* reject_tmp_path is an absolute path at this point,
> > + but that's no good for us. We need to convert this
> > + path to a *relative* path to use in the logfile. */
> > + tmpname = svn_path_basename (*reject_tmp_path, pool);
> > +
> > + if (is_dir)
> > + {
> > + /* Dealing with directory "path" */
> > + *reject_tmp_path = svn_wc__adm_path ("", TRUE, /* use tmp */ pool,
> > + tmpname, NULL);
>
> Formatting a little off here?
Sorry, I forget check tab vs spaces option in my editor after switched
to VC6.0. I'll fix it.
> ...
> > - }
> > - }
> > + /* This is the very first prop conflict found on this item. */
> > + SVN_ERR (open_reject_tmp_file (&reject_tmp_fp, &reject_tmp_path,
> > + full_path, adm_access, is_dir,
> > + pool));
>
> Formatting looks a touch odd here.
And this.
> > /* Append the conflict to the open tmp/PROPS/---.prej file */
> > SVN_ERR (append_prop_conflict (reject_tmp_fp,
> ...
>
--
Ivan Zhakov