On Thu, Mar 06, 2008 at 03:33:23AM +0100, Frans Pop wrote:
> > 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?
Sorry if that was not clear enough. So the screenshot is the screenshot
of a handmade revised version of the partman/choose_partition template,
available in cdebconf/src/test/align.templates.
After creating this template to see how it could look like, I have
noticed the alignment issue. IMHO, having right alignment support is a
minor improvement over no alignment support, so I would prefer to get
alignment support in-tree first.
> 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 :-/
Well… while trying to figure out where to put the needed ${!TAB} in
partman code I have discovered several things:
- My previous implementation of directives would not work, as
debconf_select currently relies on getting exactly one of the choice
it had put in ${CHOICES} as the result. Previously, I was expanding
directives everywhere which would not have worked well with that.
- debconf_select and ask_user do use several splitting tricks that
could probably be removed in favor of using Choices-C instead. The
code would be *a lot* simplier and faster as well.
- Putting columns properly will require quite a lot of work. :D
But anyway, I am not pushing changes in partman yet. I do think there is
a little bit of restructuring needed first.
> Something to keep in mind for right aligning a column is to preserve correct
> support for RTL scripts (e.g. Hebrew).
Thanks for reminding. :)
> 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.
Indeed! :)
> 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";
Ok, local tree updated with your second proposal. Here are screenshots
of the newt [1] and GTK+ [2] frontends.
I wonder if we should just not drop the dash completely: it just looks
like an inverted column separator, now that alignment is done
correctly. [3,4]
[1] http://people.debian.org/~lunar/localechooser-aligned-newt.png
[2] http://people.debian.org/~lunar/localechooser-aligned-gtk.png
[3] http://people.debian.org/~lunar/localechooser-aligned-no-dash-newt.png
[4] http://people.debian.org/~lunar/localechooser-aligned-no-dash-gtk.png
> 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.
I think you overlooked how the implementation works in the text or newt
frontend: the proposed patch is not using actually sending '\t' to any
terminals. It is just using the character internally as the delimiter
for columns, like the GTK+ needs. These delimiters are replaced by the
correct amount of spaces by strutl.c:stralign().
Cheers,
--
Jérémy Bobbio .''`.
lunar@debian.org : :Ⓐ : # apt-get install anarchism
`. `'`
`-
Attachment:
signature.asc
Description: Digital signature