[pacman-dev] [PATCH 3/3] Align titles automatically in information display

[Dave Reisner]

On Sat, Oct 17, 2015 at 07:18:25PM +0200, Pierre Neidhardt wrote:
> On 15-10-17 12:56:57, Dave Reisner wrote:
> > On Sat, Oct 17, 2015 at 06:49:14PM +0200, Pierre Neidhardt wrote:
> > > On 15-10-17 09:11:02, Dave Reisner wrote:
> > > > On Sat, Oct 17, 2015 at 12:04:11PM +0200, Pierre Neidhardt wrote:
> > > > > Signed-off-by: Pierre Neidhardt <ambrevar at gmail.com>
> > > > > +#define TITLE_MAXLEN 64
> > > > 
> > > > Surely, you examined the POT files to determine this value. Could you
> > > > leave behind a comment explaining as much so that it's clear what this
> > > > relates to for future readers?
> > > 
> > > Actually I did! :) Longest title string is less than 26 characters long. But you
> > > are right, I will add a comment in the next patch.
> > > 
> > > This raises a good question however: what would be a good maximum value for
> > > titles? (Titles longer than TITLE_MAXLEN get truncated.)
> > 
> > Well, it's a bit of a misfeature, IMO. Have you run a benchmark to
> > determine that heap allocating the right amount is substantially slower
> > or more complicated than keeping it on the stack? After all, this code
> > is only run once, so I'm a little skeptical that we gain anything in
> > exchange to bearing the maintenance cost of this magic value.
> 
> Agreed on performance, it's complitely negligible (2*TITLE_COUNT allocs).
> 
> But the matter is more to cleanup here. Since `dump_pkg_full` can be called

Also not terribly concerning. It's a fixed size leak which the operating
system cleans up for us on process exit. This isn't library code, but I
realize this also probably violates pacman "style" and touches a lot of
people's OCDs in funny ways.

> several times, we cannot free `titles` from here. It has to be done from one of
> the callers. Which means that 'titles' should be stored somewhere else, and thus
> passed to `make_aligned_titles` down the callstack. If we do not want to make it
> fully global, that being said.
> 
> Solutions:
> 
> * Stay static.
> * Go global.
> * Pass `tables` down the callstack.

This third option has the distinct advantage that you can now reasonably
*test* your code.

> 
> Staying static is simple and efficient IMO. TITLE_MAXLEN can be set high enough
> that it will never get rechead.
> 
> -- 
> Pierre Neidhardt
> 
> The major advances in civilization are processes that all but wreck the
> societies in which they occur.
> 		-- A.N. Whitehead