On Wednesday 05 March 2008, Jérémy Bobbio wrote:
> Well, after some hard work, it's implemented. :)
Congratulations!
> At some point, I was wondering if it would not be better just to drop
> the whole directive idea and push '\t' forward again, but I have
> actually an use-case for another directive related to "align" support:
> ${!RIGHT_ALIGN}. As you can see on the attached screenshot, partman's
> choose_partition would be a little bit better if partition sizes were
> right aligned. Work stills need to be done, though.
I guess that screenshot is with columns implemented, but not yet with the
right alignment, right?
One thing to keep in mind for implementing columns in partman's main screen
is that currently the content of columns may differ for different types of
partition tables. I noticed that myself on my sparc box when I had one disk
with msdos disklabel and another with sparc disklabel. May even still have
a screenshot of that around somewhere.
They can also differ (I think) for things like LVM and RAID.
Life is never easy :-/
Something to keep in mind for right aligning a column is to preserve correct
support for RTL scripts (e.g. Hebrew).
> The attached patch is broken into 11 different commits:
As usual I don't have any real comments on the C code, although I agree that
it does look fairly clean to my layman's eye.
> * Add support for the "align" capability to the text frontend
> * Add support for the "align" capability to the newt frontend
> * Use the "align" capability for language selection
Looking at these three patches I feel there is still room for improvement
for the columns implementation.
Take the localechooser patch:
$translation = $name .
(" " x (22 - length($name))).
- "- $translation";
+ "\${!TAB}- $translation";
What you do here is to keep the old spaces based alignment and add the tab
separator. IMO for really consistent tabs support this should be changed
to:
$translation = $name .
- (" " x (22 - length($name))).
- "- $translation";
+ "\${!TAB}- $translation";
or even:
- $translation = $name .
- (" " x (22 - length($name))).
- "- $translation";
+ $translation = $name\${!TAB}-\${!TAB}$translation";
This would of course mean that the patches for the text and newt frontends
would need to do the actual alignment themselves by adding spaces based on
the width or the actual strings.
In the current implementation adding a tab for the column for these
frontends is probably even wrong because you also leave the spaces: the tab
adds extra space.
Also, tabs are possibly not the best option for alignment as their effect
very much depends on the starting column. I would think that padding with
spaces would be easier to implement.
Anyway, great progress.
Cheers,
FJP
Attachment:
signature.asc
Description: This is a digitally signed message part.