[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[RFC] Column alignment in debconf {multi,}selects, take 2



Hi!

These days, I have been working on column support for cdebconf again.
I had previously submitted a patch [1] which was not included due to
concerns expressed about using tab ('\t', 0x09 in ASCII) as the field
separator. [2]

[1] http://lists.debian.org/debian-boot/2007/10/msg00400.html
[2] http://lists.debian.org/debian-boot/2007/10/msg00401.html
[3] http://lists.debian.org/debian-boot/2007/10/msg00482.html

The last proposal to overcome the issue was Frans' idea [3], supported by
Christian to add "debconf directives".  They would look something like
${!TAB} and have the advantage that they do not need to be escaped as
${…} is already special cased.

Well, after some hard work, it's implemented. :)

Directives have proven far more difficult than it seemed at first look.
I have took three different paths (thanks "git branch") before getting
it right.  The proposed implementation is not the most efficient, but
the code looks good and the API for frontends seems flexible enough and
I am pretty happy with that.

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.

As the column separator, I have decided to use ${!TAB} instead of an
other directive name as implementing this feature in the GTK+ frontend
proved difficult enough and the current approach really requires the use
of '\t' internally.  Other frontends also use '\t' internally, even if
that could be more easily changed now.

The attached patch is broken into 11 different commits:
 * Move variable expansion to strexpand() in strutl.c
 * Add support for "debconf directives", written ${!DIRECTIVE}
 * Add a new "align" capability to the protocol
 * Add tests for the "align" capability
 * Move strpad() from text frontend to strutl.{c,h}
 * Add stralign() to strutl.{c,h}
 * Add support for the "align" capability to the text frontend
 * Add support for the "align" capability to the newt frontend
 * Add support for the "align" capability to the GTK+ frontend
 * Update cdebconf changelog
 * Use the "align" capability for language selection

Here are the stats for the whole set:
 packages/cdebconf/debian/changelog                 |   11 +
 packages/cdebconf/doc/directives.txt               |   16 ++
 packages/cdebconf/doc/modules.txt                  |    5 +
 packages/cdebconf/src/commands.c                   |   13 +-
 packages/cdebconf/src/frontend.c                   |   14 +
 packages/cdebconf/src/frontend.h                   |    4 +
 .../cdebconf/src/modules/frontend/gtk/Makefile     |    3 +-
 .../src/modules/frontend/gtk/align_text_renderer.c |  267 ++++++++++++++++++++
 .../src/modules/frontend/gtk/align_text_renderer.h |   87 +++++++
 .../src/modules/frontend/gtk/cdebconf_gtk.c        |   34 +++-
 .../src/modules/frontend/gtk/cdebconf_gtk.h        |    7 +
 .../src/modules/frontend/gtk/choice_model.c        |    6 +-
 .../src/modules/frontend/gtk/descriptions.c        |    4 +-
 .../src/modules/frontend/gtk/select_handlers.c     |  121 ++++++++--
 packages/cdebconf/src/modules/frontend/gtk/ui.c    |   20 ++-
 packages/cdebconf/src/modules/frontend/gtk/ui.h    |    2 +
 packages/cdebconf/src/modules/frontend/newt/newt.c |   75 ++++--
 packages/cdebconf/src/modules/frontend/text/text.c |   83 ++++---
 packages/cdebconf/src/question.c                   |  111 +++------
 packages/cdebconf/src/question.h                   |   19 +-
 packages/cdebconf/src/strutl.c                     |  166 ++++++++++++-
 packages/cdebconf/src/strutl.h                     |    8 +
 packages/cdebconf/src/test/align.config            |   26 ++
 packages/cdebconf/src/test/align.templates         |   22 ++
 packages/localechooser/debian/changelog            |    7 +
 packages/localechooser/debian/control              |    3 +-
 packages/localechooser/localechooser               |    4 +-
 packages/localechooser/mktemplates.language        |    2 +-
 28 files changed, 969 insertions(+), 171 deletions(-)

To try it out, you will need to build a custom netboot image with
udebs from cdebconf and localechooser.  Sorry for not being able to
provide one, being on dial-up while writing this mail.

Cheers,
-- 
Jérémy Bobbio                        .''`. 
lunar@debian.org                    : :Ⓐ  :  # apt-get install anarchism
                                    `. `'` 
                                      `-   

Attachment: future_choose_partition.png
Description: PNG image

Attachment: align_support_take_2.diff.gz
Description: Binary data

Attachment: signature.asc
Description: Digital signature


Reply to: