On 29/12/13 04:24, Simon Gomizelj wrote: > I've been working out-of-tree for a replacement for -Qi / -Si, which can > be found here: <https://github.com/vodik/alpm-dump> and I'm looking for > some feedback before I start integrating it into pacman. > > For those not familiar with why I've decided to rewrite this, the > existing code is rather clumsy and makes some assumptions: > > string_display(_("Name :"), alpm_pkg_get_name(pkg), cols); > > The header is "Name", but the padding is embedded into the string. This > forces translation writers to also put the right paddings into their > translations, something which they shouldn't be burdened with. > > Further, `string_display` actually needs to know how long "Name :" > is so it can do proper line wrapping. The following lines need to be > indented properly. This ends up being recalculated for every header. > > Instead, if we knew how wide the widest header would be ahead of time, > we no longer need to embed padding into translations and we no longer > need to calculate line lengths for each print operation. > > This is what the code in the alpm-dump repository attempts to do. > > The approach I've taken is have the printing function take an array of > const char *. Each index represents a field, like Name or Version. If > the index is NULL, its ignored, and if it has a value, it is printed > along side corresponding data from the package. > > For example: > > static const char *sync_table[LAST_ENTRY] = { > [ENTRY_REPOSITORY] = "Repository", > [ENTRY_NAME] = "Name", > [ENTRY_VERSION] = "Version", > [ENTRY_DESCRIPTION] = "Description", > [ENTRY_ARCHITECTURE] = "Architecture", > ... > }; > > Calculating the max width means iterating across this table once looking > for the longest field length. Dumping a package is then a matter of > iterating across the table, printing the stored string and then the > value that corresponds to the array index. >
So we have two tables, one for -Si and one for -Qi. What about -Sii that adds extra fields? Will you have to add if(config->info > 1) conditions? In the output only, or also in the calculation of field width? -Qii adds "Backup Files:" at the end in a a different format, so we can ignore that... What are the arrays gaining us? Removing a few if(from == ALPM_PKG_FROM_FILE) tests? Is see the disadvantages over passing a first column width to dump_pkg_full are: 1) the field order can easily become inconsistent for -Si and -Qi. alpm_dump shows this... 2) to add/remove a field, I now need to edit an additional two arrays and a enum on top of the output function. I really like the idea of precomupting the first column width, but I am seeing a decrease in maintainability with the approach involving various arrays compared to what we currently have. Can you clarify what we gain over adding: site_t dump_pkg_column_width(alpm_pkgfrom_t, int extra) and adjusting: void dump_pkg_full(alpm_pkg_t *pkg, int extra, size_t colwidth) Allan
