[Xcb] [PATCH libxcb 11/18] c_client.py: don't generate useless empty /** < */ comments
Christian Linhart
chris at DemoRecorder.com
Sun Mar 15 03:47:51 PDT 2015
Hi Ran,
Can you please regenerate this patch so it works with the current HEAD of the repo?
Thank you,
Chris
P.S.: If you don't have time, please tell me and I'll make a new patch.
On 10/29/14 08:33, Christian Linhart wrote:
> On 10/14/14 13:51, Ran Benita wrote:
>> Hi Chris, thanks for reviewing!
>>
>> On Mon, Oct 13, 2014 at 05:50:35PM +0200, Christian Linhart wrote:
>>> Some of these changes will be lots of work to merge with my pending changes.
>>> (trivial work, but lots of it)
>>>
>>> Therefore I suggest to skip this patch for now,
>>> and recreate it after my changes have been merged to upstream.
>> Sure, any of these can be skipped if they cause trouble.
> Good, thank you.
>
> When I review your patches, I keep an eye on possible merge conflicts.
> Your other changes that I have reviewed so far, look good in this respect.
>
> So far, only this change causes trouble.
>
>> Easy conflicts
>> though can be readily resolved by the one doing the merging.
> Yep.
>
> But we have to be careful because merging is always a risk to introduce new bugs.
>
> I have some special and painful experience about that from a project with a big source-code base:
>
> At some point it was decided to create a lot of branches because of some perceived advantages for QA.
>
> The result was a lot of merging of course.
>
> And it turned out that merging causes at least the following two types of bugs:
> 1. Bugs which are created by human error, i.e., when manually resolving merge conflicts.
> 2. Bugs which are created automatically:
> This can happen when there is no textual merge conflict, but a semantic merge conflict.
> In that case, the merge tool automatically performs the textual merge without flagging that as a conflict.
>
> Therefore, merging can create a code-mixture that was neither written by a human
> not read by a human, and that does not make any sense.
> But the compiler may still accept it.
>
> Of course, bugs of type 2 are harder to avoid.
>
> To mitigate the issue we made it mandatory to review all merges,
> including the code which showed no textual merge conflicts.
>
> As you can imagine, the productivity of the development department
> dropped very significantly to the point where development
> essentially came to a grinding halt.
>
> Result: change of policy to minimize the need of merging.
> Almost following a mantra like "Merging considered harmful".
>
> ***
>
> That's why I am reviewing your patches for merge-conflicts before the actual merge happens.
>
> And that's also why I wait for all pending patches merged
> into upstream before I'll post any new changes.
> (some of which I have already implemented.
> And I am using them already in the temporary private fork of XCB which I use in my product.)
>
> Chris
>
>>> Recreating this patch should be easy because it looks like global replace in the editor.
>> Yep.
> Good.
>
> I have written that on my TODO-list,
> so that we'll think about recreating this patch
> after we'll have merged all pending patches to upstream.
>
> Chris
>
>> Ran
>>
>>> Chris
>>>
>>> On 10/12/14 20:58, Ran Benita wrote:
>>>> (This does not change doxygen's output or warnings).
>>>>
>>>> Signed-off-by: Ran Benita <ran234 at gmail.com>
>>>> ---
>>>> src/c_client.py | 88 ++++++++++++++++++++++++++++-----------------------------
>>>> 1 file changed, 44 insertions(+), 44 deletions(-)
>>>>
>>>> diff --git a/src/c_client.py b/src/c_client.py
>>>> index 4c19722..c1839c7 100644
>>>> --- a/src/c_client.py
>>>> +++ b/src/c_client.py
>>>> @@ -1111,7 +1111,7 @@ def _c_serialize(context, self):
>>>> for p in params:
>>>> typespec, pointerspec, field_name = p
>>>> spacing = ' '*(maxtypelen-len(typespec)-len(pointerspec))
>>>> - param_str.append("%s%s%s %s%s /**< */" % (indent, typespec, spacing, pointerspec, field_name))
>>>> + param_str.append("%s%s%s %s%s" % (indent, typespec, spacing, pointerspec, field_name))
>>>> # insert function name
>>>> param_str[0] = "%s (%s" % (func_name, param_str[0].strip())
>>>> param_str = ["%s," % x for x in param_str]
>>>> @@ -1292,9 +1292,9 @@ def _c_iterator(self, name):
>>>> _h(' * @brief %s', self.c_iterator_type)
>>>> _h(' **/')
>>>> _h('typedef struct %s {', self.c_iterator_type)
>>>> - _h(' %s *data; /**< */', self.c_type)
>>>> - _h(' int%s rem; /**< */', ' ' * (len(self.c_type) - 2))
>>>> - _h(' int%s index; /**< */', ' ' * (len(self.c_type) - 2))
>>>> + _h(' %s *data;', self.c_type)
>>>> + _h(' int%s rem;', ' ' * (len(self.c_type) - 2))
>>>> + _h(' int%s index;', ' ' * (len(self.c_type) - 2))
>>>> _h('} %s;', self.c_iterator_type)
>>>>
>>>> _h_setlevel(1)
>>>> @@ -1310,8 +1310,8 @@ def _c_iterator(self, name):
>>>> _h(' */')
>>>> _c('')
>>>> _hc('void')
>>>> - _h('%s (%s *i /**< */);', self.c_next_name, self.c_iterator_type)
>>>> - _c('%s (%s *i /**< */)', self.c_next_name, self.c_iterator_type)
>>>> + _h('%s (%s *i);', self.c_next_name, self.c_iterator_type)
>>>> + _c('%s (%s *i)', self.c_next_name, self.c_iterator_type)
>>>> _c('{')
>>>>
>>>> if not self.fixed_size():
>>>> @@ -1351,8 +1351,8 @@ def _c_iterator(self, name):
>>>> _h(' */')
>>>> _c('')
>>>> _hc('xcb_generic_iterator_t')
>>>> - _h('%s (%s i /**< */);', self.c_end_name, self.c_iterator_type)
>>>> - _c('%s (%s i /**< */)', self.c_end_name, self.c_iterator_type)
>>>> + _h('%s (%s i);', self.c_end_name, self.c_iterator_type)
>>>> + _c('%s (%s i)', self.c_end_name, self.c_iterator_type)
>>>> _c('{')
>>>> _c(' xcb_generic_iterator_t ret;')
>>>>
>>>> @@ -1457,8 +1457,8 @@ def _c_accessors_field(self, field):
>>>> if field.type.is_simple:
>>>> _hc('')
>>>> _hc('%s', field.c_field_type)
>>>> - _h('%s (const %s *R /**< */);', field.c_accessor_name, c_type)
>>>> - _c('%s (const %s *R /**< */)', field.c_accessor_name, c_type)
>>>> + _h('%s (const %s *R);', field.c_accessor_name, c_type)
>>>> + _c('%s (const %s *R)', field.c_accessor_name, c_type)
>>>> _c('{')
>>>> if field.prev_varsized_field is None:
>>>> _c(' return (%s *) (R + 1);', field.c_field_type)
>>>> @@ -1475,8 +1475,8 @@ def _c_accessors_field(self, field):
>>>> return_type = '%s *' % field.c_field_type
>>>>
>>>> _hc(return_type)
>>>> - _h('%s (const %s *R /**< */);', field.c_accessor_name, c_type)
>>>> - _c('%s (const %s *R /**< */)', field.c_accessor_name, c_type)
>>>> + _h('%s (const %s *R);', field.c_accessor_name, c_type)
>>>> + _c('%s (const %s *R)', field.c_accessor_name, c_type)
>>>> _c('{')
>>>> if field.prev_varsized_field is None:
>>>> _c(' return (%s) (R + 1);', return_type)
>>>> @@ -1562,8 +1562,8 @@ def _c_accessors_list(self, field):
>>>> _hc('')
>>>> _hc('%s *', field.c_field_type)
>>>>
>>>> - _h('%s (%s /**< */);', field.c_accessor_name, params[idx][0])
>>>> - _c('%s (%s /**< */)', field.c_accessor_name, params[idx][0])
>>>> + _h('%s (%s);', field.c_accessor_name, params[idx][0])
>>>> + _c('%s (%s)', field.c_accessor_name, params[idx][0])
>>>>
>>>> _c('{')
>>>> if switch_obj is not None:
>>>> @@ -1586,14 +1586,14 @@ def _c_accessors_list(self, field):
>>>> _hc('')
>>>> _hc('int')
>>>> if switch_obj is not None:
>>>> - _hc('%s (const %s *R /**< */,', field.c_length_name, R_obj.c_type)
>>>> + _hc('%s (const %s *R,', field.c_length_name, R_obj.c_type)
>>>> spacing = ' '*(len(field.c_length_name)+2)
>>>> - _h('%sconst %s *S /**< */);', spacing, S_obj.c_type)
>>>> - _c('%sconst %s *S /**< */)', spacing, S_obj.c_type)
>>>> + _h('%sconst %s *S);', spacing, S_obj.c_type)
>>>> + _c('%sconst %s *S)', spacing, S_obj.c_type)
>>>> length = _c_accessor_get_expr(field.type.expr, fields)
>>>> else:
>>>> - _h('%s (const %s *R /**< */);', field.c_length_name, c_type)
>>>> - _c('%s (const %s *R /**< */)', field.c_length_name, c_type)
>>>> + _h('%s (const %s *R);', field.c_length_name, c_type)
>>>> + _c('%s (const %s *R)', field.c_length_name, c_type)
>>>> length = _c_accessor_get_expr(field.type.expr, fields)
>>>> _c('{')
>>>> _c(' return %s;', length)
>>>> @@ -1603,13 +1603,13 @@ def _c_accessors_list(self, field):
>>>> _hc('')
>>>> _hc('xcb_generic_iterator_t')
>>>> if switch_obj is not None:
>>>> - _hc('%s (const %s *R /**< */,', field.c_end_name, R_obj.c_type)
>>>> + _hc('%s (const %s *R,', field.c_end_name, R_obj.c_type)
>>>> spacing = ' '*(len(field.c_end_name)+2)
>>>> - _h('%sconst %s *S /**< */);', spacing, S_obj.c_type)
>>>> - _c('%sconst %s *S /**< */)', spacing, S_obj.c_type)
>>>> + _h('%sconst %s *S);', spacing, S_obj.c_type)
>>>> + _c('%sconst %s *S)', spacing, S_obj.c_type)
>>>> else:
>>>> - _h('%s (const %s *R /**< */);', field.c_end_name, c_type)
>>>> - _c('%s (const %s *R /**< */)', field.c_end_name, c_type)
>>>> + _h('%s (const %s *R);', field.c_end_name, c_type)
>>>> + _c('%s (const %s *R)', field.c_end_name, c_type)
>>>> _c('{')
>>>> _c(' xcb_generic_iterator_t i;')
>>>>
>>>> @@ -1635,13 +1635,13 @@ def _c_accessors_list(self, field):
>>>> _hc('')
>>>> _hc('%s', field.c_iterator_type)
>>>> if switch_obj is not None:
>>>> - _hc('%s (const %s *R /**< */,', field.c_iterator_name, R_obj.c_type)
>>>> + _hc('%s (const %s *R,', field.c_iterator_name, R_obj.c_type)
>>>> spacing = ' '*(len(field.c_iterator_name)+2)
>>>> - _h('%sconst %s *S /**< */);', spacing, S_obj.c_type)
>>>> - _c('%sconst %s *S /**< */)', spacing, S_obj.c_type)
>>>> + _h('%sconst %s *S);', spacing, S_obj.c_type)
>>>> + _c('%sconst %s *S)', spacing, S_obj.c_type)
>>>> else:
>>>> - _h('%s (const %s *R /**< */);', field.c_iterator_name, c_type)
>>>> - _c('%s (const %s *R /**< */)', field.c_iterator_name, c_type)
>>>> + _h('%s (const %s *R);', field.c_iterator_name, c_type)
>>>> + _c('%s (const %s *R)', field.c_iterator_name, c_type)
>>>> _c('{')
>>>> _c(' %s i;', field.c_iterator_type)
>>>>
>>>> @@ -1736,10 +1736,10 @@ def _c_complex(self, force_packed = False):
>>>> # necessary for unserialize to work
>>>> (self.is_switch and field.type.is_switch)):
>>>> spacing = ' ' * (maxtypelen - len(field.c_field_type))
>>>> - _h('%s %s%s %s%s; /**< */', space, field.c_field_type, spacing, field.c_field_name, field.c_subscript)
>>>> + _h('%s %s%s %s%s;', space, field.c_field_type, spacing, field.c_field_name, field.c_subscript)
>>>> else:
>>>> spacing = ' ' * (maxtypelen - (len(field.c_field_type) + 1))
>>>> - _h('%s %s%s *%s%s; /**< */', space, field.c_field_type, spacing, field.c_field_name, field.c_subscript)
>>>> + _h('%s %s%s *%s%s;', space, field.c_field_type, spacing, field.c_field_name, field.c_subscript)
>>>>
>>>> if not self.is_switch:
>>>> for field in struct_fields:
>>>> @@ -1916,9 +1916,9 @@ def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_f
>>>>
>>>> spacing = ' ' * (maxtypelen - len('xcb_connection_t'))
>>>> comma = ',' if len(param_fields) else ');'
>>>> - _h('%s (xcb_connection_t%s *c /**< */%s', func_name, spacing, comma)
>>>> + _h('%s (xcb_connection_t%s *c%s', func_name, spacing, comma)
>>>> comma = ',' if len(param_fields) else ')'
>>>> - _c('%s (xcb_connection_t%s *c /**< */%s', func_name, spacing, comma)
>>>> + _c('%s (xcb_connection_t%s *c%s', func_name, spacing, comma)
>>>>
>>>> func_spacing = ' ' * (len(func_name) + 2)
>>>> count = len(param_fields)
>>>> @@ -1931,10 +1931,10 @@ def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_f
>>>> c_pointer = '*'
>>>> spacing = ' ' * (maxtypelen - len(c_field_const_type))
>>>> comma = ',' if count else ');'
>>>> - _h('%s%s%s %s%s /**< */%s', func_spacing, c_field_const_type,
>>>> + _h('%s%s%s %s%s%s', func_spacing, c_field_const_type,
>>>> spacing, c_pointer, field.c_field_name, comma)
>>>> comma = ',' if count else ')'
>>>> - _c('%s%s%s %s%s /**< */%s', func_spacing, c_field_const_type,
>>>> + _c('%s%s%s %s%s%s', func_spacing, c_field_const_type,
>>>> spacing, c_pointer, field.c_field_name, comma)
>>>>
>>>> count = 2
>>>> @@ -2132,10 +2132,10 @@ def _c_reply(self, name):
>>>> _h(' */')
>>>> _c('')
>>>> _hc('%s *', self.c_reply_type)
>>>> - _hc('%s (xcb_connection_t%s *c /**< */,', self.c_reply_name, spacing1)
>>>> - _hc('%s%s cookie /**< */,', spacing3, self.c_cookie_type)
>>>> - _h('%sxcb_generic_error_t%s **e /**< */);', spacing3, spacing2)
>>>> - _c('%sxcb_generic_error_t%s **e /**< */)', spacing3, spacing2)
>>>> + _hc('%s (xcb_connection_t%s *c,', self.c_reply_name, spacing1)
>>>> + _hc('%s%s cookie,', spacing3, self.c_cookie_type)
>>>> + _h('%sxcb_generic_error_t%s **e);', spacing3, spacing2)
>>>> + _c('%sxcb_generic_error_t%s **e)', spacing3, spacing2)
>>>> _c('{')
>>>>
>>>> if len(unserialize_fields)>0:
>>>> @@ -2192,9 +2192,9 @@ def _c_reply_fds(self, name):
>>>> _h(' */')
>>>> _c('')
>>>> _hc('int *')
>>>> - _hc('%s (xcb_connection_t%s *c /**< */,', self.c_reply_fds_name, spacing1)
>>>> - _h('%s%s *reply /**< */);', spacing3, self.c_reply_type)
>>>> - _c('%s%s *reply /**< */)', spacing3, self.c_reply_type)
>>>> + _hc('%s (xcb_connection_t%s *c,', self.c_reply_fds_name, spacing1)
>>>> + _h('%s%s *reply);', spacing3, self.c_reply_type)
>>>> + _c('%s%s *reply)', spacing3, self.c_reply_type)
>>>> _c('{')
>>>>
>>>> _c(' return xcb_get_reply_fds(c, reply, sizeof(%s) + 4 * reply->length);', self.c_reply_type)
>>>> @@ -2221,7 +2221,7 @@ def _c_cookie(self, name):
>>>> _h(' * @brief %s', self.c_cookie_type)
>>>> _h(' **/')
>>>> _h('typedef struct %s {', self.c_cookie_type)
>>>> - _h(' unsigned int sequence; /**< */')
>>>> + _h(' unsigned int sequence;')
>>>> _h('} %s;', self.c_cookie_type)
>>>>
>>>> def _man_request(self, name, cookie_type, void, aux):
>>>> @@ -2304,7 +2304,7 @@ def _man_request(self, name, cookie_type, void, aux):
>>>> else:
>>>> spacing = ' ' * (maxtypelen - (len(field.c_field_type) + 1))
>>>> f.write('ELSE %s = %s\n' % (field.c_field_type, field.c_field_name))
>>>> - #_h('%s %s%s *%s%s; /**< */', space, field.c_field_type, spacing, field.c_field_name, field.c_subscript)
>>>> + #_h('%s %s%s *%s%s;', space, field.c_field_type, spacing, field.c_field_name, field.c_subscript)
>>>>
>>>> if not self.is_switch:
>>>> for field in struct_fields:
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
More information about the Xcb
mailing list