On Fri, Jul 04, 2003 at 02:07:06AM +0200, Branko ??ibej wrote:
> Right! I'm finally taking the time to review this.
>
> First, a general question: Would people like to see this feature in 1.0?
> Let's see a show of hands...
Seems everybody want 1.0 out first. I'm not expecting this to be
included in 1.0. :)
Sorry for replying so late. I didn't touch svn source for a long
time, and after some tweak attempting to svn source, my memory came
back to me very slowly.
> >+/** Given an printf-like format string, return a string with proper
> >+ * information filled in.
> >+ *
> >+ * The codes of format:
> >+ *
> >+ * %a author of this revision
> >+ * %b basename of the URL of this file
> >+ * %d short format of date of this revision
> >+ * %D long format of date of this revision
> >+ * %p property value (not implemented)
> >+ * %r number of this revision
> >+ * %u URL of this file
> >
> Hm, it might be better to use %U instead of %b -- I think it's a better
> mnemonic, similar to the %d,%D pair.
Then I'd like to have %U for 'URL of the file', and %u for 'the
basename of the URL of the file'. How'd you think?
> >+ *
> >+ * All memory is allocated out of @a pool.
> >+ */
> >+svn_string_t *
> >+svn_subst_keyword_printf (const char *fmt,
> >+ const char *rev,
> >+ const char *url,
> >+ apr_time_t date,
> >+ const char *author,
> >+ apr_hash_t *props,
> >+ apr_pool_t *pool);
> >
> Ouch. If you do this, the interface of this function changes every time
> you add a non-prop value. You gen rev, url, date and author in an
> svn_entries_t anyway. Perhaps passing a hash of the revprops wouldn't
> hurt, either, then you could have %p for node props and %P for
> committed-rev props.
The purpose of svn_subst_keyword_printf() is it'll create a keyword
string from sufficient parameters. But yes, I have to admit that the
parameters are too many. Searching through source code, I found
svn_props.h. Seems props is sufficient to generate keyword string.
Maybe I can extract information from props solely.
What's the difference between node props and committed-rev props? I'm
not sure I know them...
>
> [snip]
>
> >- SVN_ERR (svn_subst_build_keywords (&tmp_keywords,
> >+ SVN_ERR (svn_wc_prop_list (&props, path, adm_access, pool));
> >+ SVN_ERR (svn_subst_build_keywords (tmp_keywords,
> > list,
> > apr_psprintf (pool, "%" SVN_REVNUM_T_FMT,
> > entry->revision),
> > entry->url,
> > entry->cmt_date,
> > entry->cmt_author,
> >+ props,
> > pool));
> >
> See? We're converting stuff from the entry struct into parameters, when
> we could just pass in the entry pointer itself.
I think the purpose of svn_subst_build_keywords() is to generate a list
of keywords, used in svn_subst_translate_stream() later. In my patch,
svn_subst_build_keywords() uses svn_subst_keyword_printf() to generate
the keyword list, demonstrating that it's functional.
> >+svn_string_t *
> >+svn_subst_keyword_printf (const char *fmt,
> >+ const char *rev,
> >+ const char *url,
> >+ apr_time_t date,
> >+ const char *author,
> >+ apr_hash_t *props,
> >+ apr_pool_t *pool)
> >
> this would be much more elegant if the format parsing, keyword value
> generation, etc. was table-based. The huge, unreadable if-elseif horror
> would just go away.
I'll see if anything I can do. :)
plasma
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jul 10 05:24:41 2003