dselect: Rework columns code

Add a new struct to hold each column data, and helper functions to
handle it, so that we can easily add new columns w/o needing to have
to manually track the current and previous column width and similar.

debian/changelog

@@ -8,6 +8,7 @@ dpkg (1.17.12) UNRELEASED; urgency=low
     - Add a short description to each virtual field in dpkg-query(1), and
       mention the version they got introduced in dpkg.
     - Fix formatting of last paragraphs inside dselect(1) --color description.
+  * Rework dselect columns code, to make it easier to maintain and read.
 
   [ Updated programs translations ]
   * German (Sven Joachim).

dselect/baselist.cc

@@ -99,6 +99,44 @@ void baselist::setupsigwinch() {
     ohshite(_("failed to set new SIGWINCH sigact"));
 }
 
+void
+baselist::add_column(column &col, const char *title, int width)
+{
+  col.title = title;
+  col.x = col_cur_x;
+  col.width = width;
+
+  col_cur_x += col.width + gap_width;
+}
+
+void
+baselist::end_column(column &col, const char *title)
+{
+  col.title = title;
+  col.x = col_cur_x;
+  col.width = total_width - col.x;
+
+  col_cur_x += col.width + gap_width;
+}
+
+void
+baselist::draw_column_head(column &col)
+{
+  mvwaddnstr(colheadspad, 0, col.x, col.title, col.width);
+}
+
+void
+baselist::draw_column_sep(column &col, int y)
+{
+  mvwaddch(listpad, y, col.x - 1, ' ');
+}
+
+void
+baselist::draw_column_item(column &col, int y, const char *item)
+{
+  mvwprintw(listpad, y, col.x, "%-*.*s", col.width, col.width, item);
+}
+
 void baselist::setheights() {
   int y= ymax - (title_height + colheads_height + thisstate_height);
   assert(y>=1);
@@ -218,6 +256,7 @@ void baselist::enddisplay() {
   delwin(infopad);
   wmove(stdscr,ymax,0); wclrtoeol(stdscr);
   listpad = nullptr;
+  col_cur_x = 0;
 }
 
 void baselist::redrawall() {
@@ -241,6 +280,7 @@ baselist::baselist(keybindings *kb) {
   bindings= kb;
   nitems= 0;
 
+  col_cur_x = 0;
   gap_width = 1;
   total_width = max(TOTAL_LIST_WIDTH, COLS);
 

dselect/dselect.h

@@ -62,6 +62,15 @@ enum screenparts {
 	numscreenparts,
 };
 
+struct column {
+	column(): title(nullptr), x(0), width(0) {};
+	void blank() { title = nullptr; x = 0; width = 0; };
+
+	const char *title;
+	int x;
+	int width;
+};
+
 class baselist {
 protected:
   // Screen dimensions &c.
@@ -73,8 +82,15 @@ protected:
   int part_attr[numscreenparts];
 
   int gap_width;
+  int col_cur_x;
   int total_width;
 
+  void add_column(column &col, const char *title, int width);
+  void end_column(column &col, const char *title);
+  void draw_column_head(column &col);
+  void draw_column_sep(column &col, int y);
+  void draw_column_item(column &col, int y, const char *item);
+
   // (n)curses stuff
   WINDOW *listpad, *infopad, *colheadspad, *thisstatepad;
   WINDOW *titlewin, *whatinfowin, *querywin;

dselect/methlist.cc

@@ -60,11 +60,11 @@ void methodlist::setheights() {
 void methodlist::setwidths() {
   debug(dbg_general, "methodlist[%p]::setwidths()", this);
 
-  status_width= 1;
-  name_width= 14;
-  name_column= status_width + gap_width;
-  description_column= name_column + name_width + gap_width;
-  description_width= total_width - description_column;
+  col_cur_x = 0;
+
+  add_column(col_status, "  ", 1);
+  add_column(col_name, _("Abbrev."), 14);
+  end_column(col_desc, _("Description"));
 }
 
 void methodlist::redrawtitle() {
@@ -93,10 +93,11 @@ void methodlist::redraw1itemsel(int index, int selected) {
   mvwaddch(listpad,index,0,
            table[index] == coption ? '*' : ' ');
   wattrset(listpad, part_attr[selected ? listsel : list]);
-  mvwprintw(listpad,index,name_column-1, " %-*.*s ",
-            name_width, name_width, table[index]->name);
+  draw_column_sep(col_name, index);
+  draw_column_item(col_name, index, table[index]->name);
 
-  i= description_width;
+  draw_column_sep(col_desc, index);
+  i = col_desc.width;
   p= table[index]->summary ? table[index]->summary : "";
   while (i>0 && *p && *p != '\n') {
     waddch(listpad,*p);
@@ -112,9 +113,9 @@ void methodlist::redrawcolheads() {
   if (colheads_height) {
     wattrset(colheadspad, part_attr[colheads]);
     mywerase(colheadspad);
-    mvwaddstr(colheadspad,0,0, "  ");
-    mvwaddnstr(colheadspad,0,name_column, _("Abbrev."), name_width);
-    mvwaddnstr(colheadspad,0,description_column, _("Description"), description_width);
+    draw_column_head(col_status);
+    draw_column_head(col_name);
+    draw_column_head(col_desc);
   }
   refreshcolheads();
 }

dselect/method.h

@@ -51,8 +51,9 @@ struct dselect_option {
 
 class methodlist : public baselist {
 protected:
-  int status_width, name_width, description_width;
-  int name_column, description_column;
+  column col_status;
+  column col_name;
+  column col_desc;
 
   // Table of methods
   struct dselect_option **table;

dselect/pkgdisplay.cc

@@ -132,56 +132,43 @@ static int maximumstring(const char *const *array) {
 void packagelist::setwidths() {
   debug(dbg_general, "packagelist[%p]::setwidths()", this);
 
+  col_cur_x = 0;
+
   if (verbose) {
-    status_hold_width= 9;
-    status_status_width= maximumstring(statusstrings);
-    status_want_width= maximumstring(wantstrings);
-    status_width= status_hold_width+status_status_width+status_want_width*2+3;
-    priority_width= 8;
-    package_width= 16;
+    add_column(col_status_hold, _("Error"), 9);
+    add_column(col_status_status, _("Installed?"), maximumstring(statusstrings));
+    add_column(col_status_old_want, _("Old mark"), maximumstring(wantstrings));
+    add_column(col_status_new_want, _("Marked for"), maximumstring(wantstrings));
   } else {
-    status_width= 4;
-    priority_width= 3;
-    package_width= 12;
+    add_column(col_status, _("EIOM"), 4);
   }
-  section_width= 8;
 
-  if (sortorder == so_section) {
-    section_column= status_width + gap_width;
-    priority_column= section_column + section_width + gap_width;
-    package_column= priority_column + priority_width + gap_width;
-  } else {
-    priority_column= status_width + gap_width;
-    section_column= priority_column + priority_width + gap_width;
-    package_column= section_column + section_width + gap_width;
-  }
+  if (sortorder == so_section)
+    add_column(col_section, _("Section"), 8);
+  add_column(col_priority, _("Priority"), verbose ? 8 : 3);
+  if (sortorder != so_section)
+    add_column(col_section, _("Section"), 8);
 
-  int versiondescriptioncolumn= package_column + package_width + gap_width;
+  add_column(col_package, _("Package"), verbose ? 16 : 12);
 
   switch (versiondisplayopt) {
   case vdo_none:
-    versioninstalled_column= versioninstalled_width= 0;
-    versionavailable_column= versionavailable_width= 0;
-    description_column= versiondescriptioncolumn;
+    col_versioninstalled.blank();
+    col_versionavailable.blank();
     break;
   case vdo_available:
-    versioninstalled_column= versioninstalled_width= 0;
-    versionavailable_column= versiondescriptioncolumn;
-    versionavailable_width= 11;
-    description_column= versionavailable_column + versionavailable_width + gap_width;
+    col_versioninstalled.blank();
+    add_column(col_versionavailable, _("Avail.ver"), 11);
     break;
   case vdo_both:
-    versioninstalled_column= versiondescriptioncolumn;
-    versioninstalled_width= 11;
-    versionavailable_column= versioninstalled_column + versioninstalled_width +gap_width;
-    versionavailable_width= versioninstalled_width;
-    description_column= versionavailable_column + versionavailable_width + gap_width;
+    add_column(col_versioninstalled, _("Inst.ver"), 11);
+    add_column(col_versionavailable, _("Avail.ver"), 11);
     break;
   default:
     internerr("unknown versiondisplayopt %d", versiondisplayopt);
   }
 
-  description_width= total_width - description_column;
+  end_column(col_description, _("Description"));
 }
 
 void packagelist::redrawtitle() {

dselect/pkglist.h

@@ -88,13 +88,19 @@ struct perpackagestate {
 
 class packagelist : public baselist {
 protected:
-  int status_width, section_width, priority_width;
-  int package_width, versioninstalled_width, versionavailable_width, description_width;
-  int section_column, priority_column, versioninstalled_column;
-  int versionavailable_column, package_column, description_column;
+  column col_status;
+  column col_section;
+  column col_priority;
+  column col_package;
+  column col_versioninstalled;
+  column col_versionavailable;
+  column col_description;
 
   // Only used when ‘verbose’ is set
-  int status_hold_width, status_status_width, status_want_width;
+  column col_status_hold;
+  column col_status_status;
+  column col_status_old_want;
+  column col_status_new_want;
 
   // Table of packages
   struct perpackagestate *datatable;

dselect/pkgtop.cc

@@ -117,7 +117,7 @@ void packagelist::redrawthisstate() {
   if (table[cursorline]->pkg->set->name) {
     sprintf(buf,
             _("%-*s %s%s%s;  %s (was: %s).  %s"),
-            package_width,
+            col_package.width,
             table[cursorline]->pkg->set->name,
             gettext(statusstrings[table[cursorline]->pkg->status]),
             ((eflagstrings[table[cursorline]->pkg->eflag][0]==' ') &&
@@ -147,28 +147,31 @@ void packagelist::redraw1itemsel(int index, int selected) {
 
   if (pkg->set->name) {
     if (verbose) {
-      mvwprintw(listpad, screenline, 0, "%-*.*s ",
-                status_hold_width, status_hold_width,
-                gettext(eflagstrings[pkg->eflag]));
-      wprintw(listpad, "%-*.*s ",
-              status_status_width, status_status_width,
-              gettext(statusstrings[pkg->status]));
-      wprintw(listpad, "%-*.*s ",
-              status_want_width, status_want_width,
+      draw_column_item(col_status_hold, screenline,
+                       gettext(eflagstrings[pkg->eflag]));
+
+      draw_column_sep(col_status_status, screenline);
+      draw_column_item(col_status_status, screenline,
+                       gettext(statusstrings[pkg->status]));
+
+      draw_column_sep(col_status_old_want, screenline);
+      draw_column_item(col_status_old_want, screenline,
               /* FIXME: keep this? */
               /*table[index]->original == table[index]->selected ? "(same)"
               : */gettext(wantstrings[table[index]->original]));
+
+      draw_column_sep(col_status_new_want, screenline);
       wattrset(listpad, part_attr[selected ? selstatesel : selstate]);
-      wprintw(listpad, "%-*.*s",
-              status_want_width, status_want_width,
-              gettext(wantstrings[table[index]->selected]));
+      draw_column_item(col_status_new_want, screenline,
+                       gettext(wantstrings[table[index]->selected]));
+
       wattrset(listpad, part_attr[selected ? listsel : list]);
-      waddch(listpad, ' ');
 
-      mvwprintw(listpad, screenline, priority_column - 1, " %-*.*s",
-                priority_width, priority_width,
-                pkg->priority == PKG_PRIO_OTHER ? pkg->otherpriority :
-                gettext(prioritystrings[pkg->priority]));
+      draw_column_sep(col_priority, screenline);
+      draw_column_item(col_priority, screenline,
+                       pkg->priority == PKG_PRIO_OTHER ?
+                       pkg->otherpriority :
+                       gettext(prioritystrings[pkg->priority]));
     } else {
       mvwaddch(listpad, screenline, 0, eflagchars[pkg->eflag]);
       waddch(listpad, statuschars[pkg->status]);
@@ -181,44 +184,47 @@ void packagelist::redraw1itemsel(int index, int selected) {
       waddch(listpad, wantchars[table[index]->selected]);
       wattrset(listpad, part_attr[selected ? listsel : list]);
 
-      wmove(listpad, screenline, priority_column - 1);
+      wmove(listpad, screenline, col_priority.x - 1);
       waddch(listpad, ' ');
       if (pkg->priority == PKG_PRIO_OTHER) {
-        for (i=priority_width, p=pkg->otherpriority;
+        for (i = col_priority.width, p = pkg->otherpriority;
              i > 0 && *p;
              i--, p++)
           waddch(listpad, tolower(*p));
         while (i-- > 0) waddch(listpad,' ');
       } else {
-        wprintw(listpad, "%-*.*s", priority_width, priority_width,
+        wprintw(listpad, "%-*.*s", col_priority.width, col_priority.width,
                 gettext(priorityabbrevs[pkg->priority]));
       }
     }
 
-    mvwprintw(listpad, screenline, section_column - 1, " %-*.*s",
-              section_width, section_width,
-              pkg->section ? pkg->section : "?");
+    draw_column_sep(col_section, screenline);
+    draw_column_item(col_section, screenline,
+                     pkg->section ? pkg->section : "?");
+
+    draw_column_sep(col_package, screenline);
+    draw_column_item(col_package, screenline,
+                     pkg->set->name);
 
-    mvwprintw(listpad, screenline, package_column - 1, " %-*.*s ",
-              package_width, package_width, pkg->set->name);
+    waddch(listpad, ' ');
 
-    if (versioninstalled_width)
-      mvwprintw(listpad, screenline, versioninstalled_column, "%-*.*s ",
-                versioninstalled_width, versioninstalled_width,
-                versiondescribe(&pkg->installed.version, vdew_nonambig));
-    if (versionavailable_width) {
+    if (col_versioninstalled.width) {
+      draw_column_item(col_versioninstalled, screenline,
+                       versiondescribe(&pkg->installed.version, vdew_nonambig));
+      waddch(listpad, ' ');
+    }
+    if (col_versionavailable.width) {
       if (dpkg_version_is_informative(&pkg->available.version) &&
           dpkg_version_compare(&pkg->available.version,
                                &pkg->installed.version) > 0)
         wattrset(listpad, part_attr[selected ? selstatesel : selstate]);
-      mvwprintw(listpad, screenline, versionavailable_column, "%-*.*s",
-                versionavailable_width, versionavailable_width,
-                versiondescribe(&pkg->available.version, vdew_nonambig));
+      draw_column_item(col_versionavailable, screenline,
+                       versiondescribe(&pkg->available.version, vdew_nonambig));
       wattrset(listpad, part_attr[selected ? listsel : list]);
       waddch(listpad,' ');
     }
 
-    i= description_width;
+    i = col_description.width;
     p= info->description ? info->description :
        pkg->installed.description ? pkg->installed.description : "";
     while (i>0 && *p && *p != '\n') { waddnstr(listpad,p,1); i--; p++; }
@@ -259,38 +265,26 @@ void packagelist::redrawcolheads() {
     mywerase(colheadspad);
     if (verbose) {
       wmove(colheadspad,0,0);
-      for (int i=0; i<status_width-status_want_width; i++) waddch(colheadspad,'.');
-      mvwaddnstr(colheadspad,0,
-                 0,
-                 _("Error"),
-                 status_hold_width);
-      mvwaddnstr(colheadspad,0,
-                 status_hold_width+1,
-                 _("Installed?"),
-                 status_status_width);
-      mvwaddnstr(colheadspad,0,
-                 status_hold_width+status_status_width+2,
-                 _("Old mark"),
-                 status_want_width);
-      mvwaddnstr(colheadspad,0,
-                 status_hold_width+status_status_width+status_want_width+3,
-                 _("Marked for"),
-                 status_want_width);
+      for (int i = 0; i < col_status_old_want.width; i++)
+        waddch(colheadspad, '.');
+      draw_column_head(col_status_hold);
+      draw_column_head(col_status_status);
+      draw_column_head(col_status_old_want);
+      draw_column_head(col_status_new_want);
     } else {
-      mvwaddstr(colheadspad,0,0, _("EIOM"));
+      draw_column_head(col_status);
     }
-    mvwaddnstr(colheadspad,0,section_column, _("Section"), section_width);
-    mvwaddnstr(colheadspad,0,priority_column, _("Priority"), priority_width);
-    mvwaddnstr(colheadspad,0,package_column, _("Package"), package_width);
-
-    if (versioninstalled_width)
-      mvwaddnstr(colheadspad,0,versioninstalled_column,
-                 _("Inst.ver"),versioninstalled_width);
-    if (versionavailable_width)
-      mvwaddnstr(colheadspad,0,versionavailable_column,
-                 _("Avail.ver"),versionavailable_width);
-
-    mvwaddnstr(colheadspad,0,description_column, _("Description"), description_width);
+
+    draw_column_head(col_section);
+    draw_column_head(col_priority);
+    draw_column_head(col_package);
+
+    if (col_versioninstalled.width)
+      draw_column_head(col_versioninstalled);
+    if (col_versionavailable.width)
+      draw_column_head(col_versionavailable);
+
+    draw_column_head(col_description);
   }
   refreshcolheads();
 }