On 02/29/2016 03:47 PM, Aurelio Colosimo wrote:
>> The merge directive works as expected, your change history is in there
>> :-) The coding standard looks good too, with two nits:
>>
>> +NO_TRACE static const char *cmdtab_enum(const char *name,
>> + const char **h, void **ctx)
>> {
>> + link_t **startpos = (link_t**)ctx;
>>
>> The last line is not a continuation line, so it should come with a TAB
>> instead of four spaces.
>
> Please can you explain what do you mean for a "continuation line"? It
> seemed to me that, normally, "const char **h" would go on the previous
> line, but I'm "continuing" the parameters list in the line below, that's
> why I used spaces.
The second line of the following is a continuation line:
NO_TRACE static const char *cmdtab_enum(const char *name,
const char **h, void **ctx)
in HelenOS, we indent continuation lines by 4 spaces relative to the
indentation level of the original line.
My comment, however, was about this line:
link_t **startpos = (link_t**)ctx;
which is not a continuation line and needs to be indented by one tab.
>> The second nit is that sometimes you omit whitespace around *, for
>> example here:
>>
>> + const char*(*hints_enum)(const char *, const char **, void **);
>
> Ok, I will check and fix them. About this specific function pointer, I
> wonder whether you prefer a typedef for it, e.g.:
>
> typedef const char *(*hints_enum_func_t)(const char *, const char **,
> void **);
Let's do it.
> Ok, so I think I will go on working on it, then I will propose another
> set of patches, implementing the complete feature, to be applied in
> place of the ones I sent last week.
Ok, thanks for working on this.
Jakub
_______________________________________________
HelenOS-devel mailing list
HelenOS-devel@lists.modry.cz
http://lists.modry.cz/listinfo/helenos-devel
