On 15-10-15 21:11:22, Allan McRae wrote:
https://patchwork.archlinux.org/patch/1131/ https://patchwork.archlinux.org/patch/1132/ https://patchwork.archlinux.org/patch/1133/
Mostly the last one I think... It has been more than three years since I looked at this.
Are you already there, in 2016? :D Jokes aside, I had a look at the code + the patch set. Patch 2 and 3 do some minor refactoring: * Compute 'len' early. I don't see the point, it is less clear and wastes some cycles. * Move some *_display function from util.c to package.c and make them static. This makes the code base less uniform. I don't think that the potential (?) code optimization is worth it. * Rename 'cols' to maxcols'. Why not. * Replace the indentation loop with a printf("%-*s"...). Why not. Patch 4 does more important refactoring. It moves a lot of code out of util.c to the new termio.c. Some functions have been added, which possibly factor some redundant parts. (I would need to study the code a little bit more to make sure of this). The resulting code base is bigger: 240 insertions(+), 154 deletions(-), so I am not sure where the gain is. Now: * I could not see anything related to alighment. I suppose the patch set was laying the ground work for further improvements. * Do you think it is worth basing future work on this patch set? I would say no at first glance. * I can see 2 ways to fix the alignment issue: 1) Remove the (regex) " *:" in every string including .po's and pad spaces dynamically. The current .po maintenance seems rather shallow, and this will keep alignment broken for most locales for a while. (Actually, it will make it worse!) 2) Keep existing strings unchanged, ignore everything that matches " *:" and align from there. This is a bit dirty in the sense that this special case will remain even after the " *:" are gone from the .po. I'd rather go for 1). Do you think it would be easy to batch process all .po's on Transifex? I have absolutely no clue, I've never used it. -- Pierre Neidhardt