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 Shahaf <d....@daniel.shahaf.name> on 2019/01/04 14:20:42 UTC
Re: svn commit: r1850344 - in /subversion/trunk/subversion:
bindings/cxx/include/svnxx/ bindings/cxx/src/ include/
brane@apache.org wrote on Fri, 04 Jan 2019 10:38 +0000:
> Move (some of the) standalone types into separate implementation headers
> so that SVN++ can use them directly without exposing APR or other dependencies.
> +++ subversion/trunk/subversion/include/svn_opt_impl.h Fri Jan 4
> 10:38:53 2019
> @@ -0,0 +1,83 @@
> + * @file svn_opt.h
> + * @brief Option and argument parsing for Subversion command lines
> + * (common implementation)
> + */
> +
> +/* NOTE:
> + * This file *must not* include or depend on any other header except
> + * the C standard library headers.
> + */
Could the comment also explain the rationale for the restriction it imposes?
> +
> +#ifndef SVN_OPT_IMPL_H
> +#define SVN_OPT_IMPL_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif /* __cplusplus */
Cheers,
Daniel
Re: svn commit: r1850344 - in /subversion/trunk/subversion:
bindings/cxx/include/svnxx/ bindings/cxx/src/ include/
Posted by Branko Čibej <br...@apache.org>.
On 04.01.2019 15:48, Daniel Shahaf wrote:
> Branko Čibej wrote on Fri, 04 Jan 2019 15:42 +0100:
>> On 04.01.2019 15:35, Daniel Shahaf wrote:
>>> Branko Čibej wrote on Fri, 04 Jan 2019 15:31 +0100:
>>>> On 04.01.2019 15:20, Daniel Shahaf wrote:
>>>>> brane@apache.org wrote on Fri, 04 Jan 2019 10:38 +0000:
>>>>>> Move (some of the) standalone types into separate implementation headers
>>>>>> so that SVN++ can use them directly without exposing APR or other dependencies.
>>>>>> +++ subversion/trunk/subversion/include/svn_opt_impl.h Fri Jan 4
>>>>>> 10:38:53 2019
>>>>>> @@ -0,0 +1,83 @@
>>>>>> + * @file svn_opt.h
>>>>>> + * @brief Option and argument parsing for Subversion command lines
>>>>>> + * (common implementation)
>>>>>> + */
>>>>>> +
>>>>>> +/* NOTE:
>>>>>> + * This file *must not* include or depend on any other header except
>>>>>> + * the C standard library headers.
>>>>>> + */
>>>>> Could the comment also explain the rationale for the restriction it imposes?
>>>> No, not in every "impl" header we happen to create. But it is documented
>>>> in subversion/bindings/cxx/README, as one of the SVN++ design goals.
>>> Shouldn't this be documented in the C API's documentation too, at least
>>> by reference? Someone working on the C headers in a year or three might
>>> not think to look in the C++ bindings for design choices of the C API.
>> If you have an idea how to do that without boring repetition, please go
>> ahead.
> We could add a paragraph to HACKING, or possibly to a single global
> doxygen file, explaining the _impl.h convention, rather than repeating
> the explanation in each individual file.
A Doxygen file wouldn't work since we don't use generated docs for
developer guidelines, but a paragraph in HACKING about C++ bindings and
similar policy would work.
-- Brane
Re: svn commit: r1850344 - in /subversion/trunk/subversion:
bindings/cxx/include/svnxx/ bindings/cxx/src/ include/
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Fri, 04 Jan 2019 15:42 +0100:
> On 04.01.2019 15:35, Daniel Shahaf wrote:
> > Branko Čibej wrote on Fri, 04 Jan 2019 15:31 +0100:
> >> On 04.01.2019 15:20, Daniel Shahaf wrote:
> >>> brane@apache.org wrote on Fri, 04 Jan 2019 10:38 +0000:
> >>>> Move (some of the) standalone types into separate implementation headers
> >>>> so that SVN++ can use them directly without exposing APR or other dependencies.
> >>>> +++ subversion/trunk/subversion/include/svn_opt_impl.h Fri Jan 4
> >>>> 10:38:53 2019
> >>>> @@ -0,0 +1,83 @@
> >>>> + * @file svn_opt.h
> >>>> + * @brief Option and argument parsing for Subversion command lines
> >>>> + * (common implementation)
> >>>> + */
> >>>> +
> >>>> +/* NOTE:
> >>>> + * This file *must not* include or depend on any other header except
> >>>> + * the C standard library headers.
> >>>> + */
> >>> Could the comment also explain the rationale for the restriction it imposes?
> >> No, not in every "impl" header we happen to create. But it is documented
> >> in subversion/bindings/cxx/README, as one of the SVN++ design goals.
> > Shouldn't this be documented in the C API's documentation too, at least
> > by reference? Someone working on the C headers in a year or three might
> > not think to look in the C++ bindings for design choices of the C API.
>
> If you have an idea how to do that without boring repetition, please go
> ahead.
We could add a paragraph to HACKING, or possibly to a single global
doxygen file, explaining the _impl.h convention, rather than repeating
the explanation in each individual file.
Re: svn commit: r1850344 - in /subversion/trunk/subversion:
bindings/cxx/include/svnxx/ bindings/cxx/src/ include/
Posted by Branko Čibej <br...@apache.org>.
On 04.01.2019 15:35, Daniel Shahaf wrote:
> Branko Čibej wrote on Fri, 04 Jan 2019 15:31 +0100:
>> On 04.01.2019 15:20, Daniel Shahaf wrote:
>>> brane@apache.org wrote on Fri, 04 Jan 2019 10:38 +0000:
>>>> Move (some of the) standalone types into separate implementation headers
>>>> so that SVN++ can use them directly without exposing APR or other dependencies.
>>>> +++ subversion/trunk/subversion/include/svn_opt_impl.h Fri Jan 4
>>>> 10:38:53 2019
>>>> @@ -0,0 +1,83 @@
>>>> + * @file svn_opt.h
>>>> + * @brief Option and argument parsing for Subversion command lines
>>>> + * (common implementation)
>>>> + */
>>>> +
>>>> +/* NOTE:
>>>> + * This file *must not* include or depend on any other header except
>>>> + * the C standard library headers.
>>>> + */
>>> Could the comment also explain the rationale for the restriction it imposes?
>> No, not in every "impl" header we happen to create. But it is documented
>> in subversion/bindings/cxx/README, as one of the SVN++ design goals.
> Shouldn't this be documented in the C API's documentation too, at least
> by reference? Someone working on the C headers in a year or three might
> not think to look in the C++ bindings for design choices of the C API.
If you have an idea how to do that without boring repetition, please go
ahead.
I just don't see it as all that relevant apart from the "don't do that"
header in the files. Every existing type that was moved to the new
headers already has a reference to its C++ counterpart[1]. When we
invent new types, trivial cases will be caught when (if?) their C++
wrappers are written.
-- Brane
[1] ... except svn_node_kind_t, which doesn't have a C++ counterpart yet.
Re: svn commit: r1850344 - in /subversion/trunk/subversion:
bindings/cxx/include/svnxx/ bindings/cxx/src/ include/
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Fri, 04 Jan 2019 15:31 +0100:
> On 04.01.2019 15:20, Daniel Shahaf wrote:
> > brane@apache.org wrote on Fri, 04 Jan 2019 10:38 +0000:
> >> Move (some of the) standalone types into separate implementation headers
> >> so that SVN++ can use them directly without exposing APR or other dependencies.
> >> +++ subversion/trunk/subversion/include/svn_opt_impl.h Fri Jan 4
> >> 10:38:53 2019
> >> @@ -0,0 +1,83 @@
> >> + * @file svn_opt.h
> >> + * @brief Option and argument parsing for Subversion command lines
> >> + * (common implementation)
> >> + */
> >> +
> >> +/* NOTE:
> >> + * This file *must not* include or depend on any other header except
> >> + * the C standard library headers.
> >> + */
> > Could the comment also explain the rationale for the restriction it imposes?
>
> No, not in every "impl" header we happen to create. But it is documented
> in subversion/bindings/cxx/README, as one of the SVN++ design goals.
Shouldn't this be documented in the C API's documentation too, at least
by reference? Someone working on the C headers in a year or three might
not think to look in the C++ bindings for design choices of the C API.
Re: svn commit: r1850344 - in /subversion/trunk/subversion:
bindings/cxx/include/svnxx/ bindings/cxx/src/ include/
Posted by Branko Čibej <br...@apache.org>.
On 04.01.2019 15:20, Daniel Shahaf wrote:
> brane@apache.org wrote on Fri, 04 Jan 2019 10:38 +0000:
>> Move (some of the) standalone types into separate implementation headers
>> so that SVN++ can use them directly without exposing APR or other dependencies.
>> +++ subversion/trunk/subversion/include/svn_opt_impl.h Fri Jan 4
>> 10:38:53 2019
>> @@ -0,0 +1,83 @@
>> + * @file svn_opt.h
>> + * @brief Option and argument parsing for Subversion command lines
>> + * (common implementation)
>> + */
>> +
>> +/* NOTE:
>> + * This file *must not* include or depend on any other header except
>> + * the C standard library headers.
>> + */
> Could the comment also explain the rationale for the restriction it imposes?
No, not in every "impl" header we happen to create. But it is documented
in subversion/bindings/cxx/README, as one of the SVN++ design goals.
-- Brane