[pacman-dev] [PATCH v2 3/3] Align titles automatically in information display
ambrevar at gmail.com
Tue Oct 20 12:11:41 UTC 2015
On 15-10-20 06:58:58, Dave Reisner wrote:
> On Tue, Oct 20, 2015 at 10:29:43AM +0200, Pierre Neidhardt wrote:
> > On 15-10-19 17:22:55, Dave Reisner wrote:
> > > I'd also love to claim that this needs to share the same prefix as the enums
> > > it belongs to (and have an underscore prefix), but there's not really any
> > > prevailing coding style in ALPM that I can lean on to "enforce" that.
> > Agreed, but I prefer keeping underscore prefixes for library code. This is not
> > ALPM but the pacman frontend if this is what you meant.
> I've no idea why you would suddenly change coding style for internal
> library code versus frontend code.
I believe there is a misunderstanding here. To me, underscore prefixed variables
are used in libraries as a mean to tell the calling programs "don't use me, I'm
My patch only changes 'package.c' which is part of the frontend code. Following
the aforementioned coding style, I would rather not prefix any variable with an
That is really a nit though, if you really like it better, I don't mind
> > > > +};
> > > > +
> > > > +static char titles[TITLE_COUNT][TITLE_MAXLEN * sizeof(wchar_t)];
> I do wonder if it isn't best to also only use _T_MAX here and elsewhere
> in the code where you traverse the arrays use:
> And perhaps introduces an ARRAYSIZE or ELEMENTSOF macro to do this for
> us (we'd make use it elsewhere in the lib and frontend). This is safer
> and clearer to the reader that you're traversing the precise bounds of
> the array, not some max index that you "hope" lines up with the bound.
Agreed, plus this is idiomatic enough in C. But! From HACKING:
> 6. The sizeof() operator should accept a type, not a value. (TODO: in certain
> cases, it may be better- should this be a set guideline? Read "The Practice
> of Programming")
I must say I agree with the TODO. Shall we update this? From your first review,
you asked for 2 changes that go againt paragraph 6:
static const size_t title_suffix_len = sizeof(title_suffix);
wcstombs(titles[i], wbuf[i], sizeof(wbuf[i]));
Besides, I have seen a few other places in the current code where §6 is being
Should we agree on this, shall we place the ARRAYSIZE macro in util-common.h?
Other typical names include LENGTH, SIZE, COUNT_OF, ELEMENTSOF. I like ARRAYSIZE
better as it makes clear it must be an array.
An optimist is a guy that has never had much experience.
-- Don Marquis
More information about the pacman-dev