From 1404591baa5849f97a69a1090daf24206d5812ca Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 27 Aug 2024 09:23:53 +0200 Subject: [PATCH 1/4] ui: replace use of deprecated user_arg by kw-only arg user_args urwid deprecated the user of the user_arg parameter of connect_signal (and might get dropped sometime). Instead, the suggested approach is to use user_args (plural!) as a kw-only argument. We have to change the signature of callbacks functions because `user_args` is passed as an argument at the beginning ; whereas `user_arg` is passed as an argument at the end. Signed-off-by: Olivier Gayot --- console_conf/ui/views/chooser.py | 4 ++-- subiquity/ui/views/error.py | 6 +++--- subiquity/ui/views/filesystem/compound.py | 6 ++++-- subiquity/ui/views/filesystem/filesystem.py | 8 ++++---- subiquity/ui/views/ssh.py | 4 ++-- subiquity/ui/views/zdev.py | 4 ++-- subiquitycore/controllers/network.py | 2 +- subiquitycore/ui/actionmenu.py | 6 ++++-- subiquitycore/ui/selector.py | 4 ++-- subiquitycore/ui/views/network.py | 4 ++-- 10 files changed, 26 insertions(+), 22 deletions(-) diff --git a/console_conf/ui/views/chooser.py b/console_conf/ui/views/chooser.py index d033c099b..6871c4ff1 100644 --- a/console_conf/ui/views/chooser.py +++ b/console_conf/ui/views/chooser.py @@ -128,7 +128,7 @@ def __init__(self, controller, systems): Action(label=act.title.capitalize(), value=act, enabled=True) ) menu = ActionMenu(actions) - connect_signal(menu, "action", self._system_action, s) + connect_signal(menu, "action", self._system_action, user_args=[s]) srow = make_action_menu_row( [ Text(s.label), @@ -162,7 +162,7 @@ def __init__(self, controller, systems): ), ) - def _system_action(self, sender, action, system): + def _system_action(self, system, sender, action): self.controller.select(system, action) def back(self, result): diff --git a/subiquity/ui/views/error.py b/subiquity/ui/views/error.py index 94d38736f..b5e756474 100644 --- a/subiquity/ui/views/error.py +++ b/subiquity/ui/views/error.py @@ -397,7 +397,7 @@ def __init__(self, app): self.report_to_row = {} self.app.error_reporter.load_reports() for report in self.app.error_reporter.reports: - connect_signal(report, "changed", self._report_changed, report) + connect_signal(report, "changed", self._report_changed, user_args=[report]) r = self.report_to_row[report] = self.row_for_report(report) rows.append(r) self.table = TablePile(rows, colspecs={1: ColSpec(can_shrink=True)}) @@ -410,7 +410,7 @@ def __init__(self, app): ] super().__init__("", widgets, 2, 2) - def open_report(self, sender, report): + def open_report(self, report, sender): self.app.add_global_overlay(ErrorReportStretchy(self.app, report, False)) def state_for_report(self, report): @@ -421,7 +421,7 @@ def state_for_report(self, report): def cells_for_report(self, report): date = report.pr.get("Date", "???") icon = ClickableIcon(date) - connect_signal(icon, "click", self.open_report, report) + connect_signal(icon, "click", self.open_report, user_args=[report]) return [ Text("["), icon, diff --git a/subiquity/ui/views/filesystem/compound.py b/subiquity/ui/views/filesystem/compound.py index 5dc81fde1..b100db160 100644 --- a/subiquity/ui/views/filesystem/compound.py +++ b/subiquity/ui/views/filesystem/compound.py @@ -98,7 +98,7 @@ def _state_change_device(self, sender, state, device): del self.devices[device] self._emit("change", self.devices) - def _select_active_spare(self, sender, value, device): + def _select_active_spare(self, device, sender, value): self.devices[device] = value self._emit("change", self.devices) @@ -151,7 +151,9 @@ def set_bound_form_field(self, bff): self.all_rows.append(Color.menu_button(TableRow([box, size]))) self.no_selector_rows.append(self.all_rows[-1]) selector = Selector(["active", "spare"]) - connect_signal(selector, "select", self._select_active_spare, device) + connect_signal( + selector, "select", self._select_active_spare, user_args=[device] + ) selector = Toggleable( UrwidPadding(Color.menu_button(selector), left=len(prefix)) ) diff --git a/subiquity/ui/views/filesystem/filesystem.py b/subiquity/ui/views/filesystem/filesystem.py index e4062cef0..14a5ea6d8 100644 --- a/subiquity/ui/views/filesystem/filesystem.py +++ b/subiquity/ui/views/filesystem/filesystem.py @@ -113,7 +113,7 @@ def __init__(self, parent): ) super().__init__(self.table) - def _mount_action(self, sender, action, mount): + def _mount_action(self, mount, sender, action): log.debug("_mount_action %s %s", action, mount) if action == "unmount": self.parent.controller.delete_mount(mount) @@ -168,7 +168,7 @@ def refresh_model_inputs(self): ] actions = [(_("Unmount"), mi.mount.can_delete(), "unmount")] menu = ActionMenu(actions) - connect_signal(menu, "action", self._mount_action, mi.mount) + connect_signal(menu, "action", self._mount_action, user_args=[mi.mount]) cells = [ Text("["), Text(path_markup), @@ -306,7 +306,7 @@ def _disk_TOGGLE_BOOT(self, disk): lambda parent, gap: PartitionStretchy(parent, gap.device, gap=gap) ) - def _action(self, sender, value, device): + def _action(self, device, sender, value): action, meth = value log.debug("_action %s %s", action, device.id) meth(device) @@ -365,7 +365,7 @@ def _action_menu_for_device(self, device): if not device_actions: return Text("") menu = ActionMenu(device_actions) - connect_signal(menu, "action", self._action, device) + connect_signal(menu, "action", self._action, user_args=[device]) return menu def refresh_model_inputs(self): diff --git a/subiquity/ui/views/ssh.py b/subiquity/ui/views/ssh.py index 57628429f..c7bba5796 100644 --- a/subiquity/ui/views/ssh.py +++ b/subiquity/ui/views/ssh.py @@ -407,11 +407,11 @@ def refresh_keys_table(self): menu, ) ) - connect_signal(menu, "action", self._action, key) + connect_signal(menu, "action", self._action, user_args=[key]) self.keys_table.set_contents(rows) - def _action(self, sender, value, key): + def _action(self, key, sender, value): if value == "delete": self.remove_key_from_table(key) self.refresh_keys_table() diff --git a/subiquity/ui/views/zdev.py b/subiquity/ui/views/zdev.py index f572232c0..39629b285 100644 --- a/subiquity/ui/views/zdev.py +++ b/subiquity/ui/views/zdev.py @@ -69,7 +69,7 @@ async def _zdev_action(self, action, zdevinfo): self.update(new_zdevinfos) self.parent.request_redraw_if_visible() - def zdev_action(self, sender, action, zdevinfo): + def zdev_action(self, zdevinfo, sender, action): run_bg_task(self._zdev_action(action, zdevinfo)) def update(self, zdevinfos): @@ -120,7 +120,7 @@ def update(self, zdevinfos): (_("Disable"), zdevinfo.on, "disable"), ] menu = ActionMenu(actions) - connect_signal(menu, "action", self.zdev_action, zdevinfo) + connect_signal(menu, "action", self.zdev_action, user_args=[zdevinfo]) cells = [ Text(zdevinfo.id), status(zdevinfo), diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py index 7bad4f336..5eee86317 100644 --- a/subiquitycore/controllers/network.py +++ b/subiquitycore/controllers/network.py @@ -585,7 +585,7 @@ async def _answers_action(self, action): table = self._action_get(action["obj"]) meth = getattr(self.ui.body, "_action_{}".format(action["action"])) action_obj = getattr(NetDevAction, action["action"]) - self.ui.body._action(None, (action_obj, meth), table) + self.ui.body._action(table, None, (action_obj, meth)) yield body = self.ui.body._w if action["action"] == "DELETE": diff --git a/subiquitycore/ui/actionmenu.py b/subiquitycore/ui/actionmenu.py index f6cea7e23..81accb95b 100644 --- a/subiquitycore/ui/actionmenu.py +++ b/subiquitycore/ui/actionmenu.py @@ -63,7 +63,9 @@ def __init__(self, parent): else: btn = Color.menu_button(ActionMenuButton(action.label)) width = max(width, len(btn.base_widget.label)) - connect_signal(btn.base_widget, "click", self.click, action.value) + connect_signal( + btn.base_widget, "click", self.click, user_args=[action.value] + ) else: label = action.label if isinstance(label, Widget): @@ -89,7 +91,7 @@ def __init__(self, parent): def close(self, sender): self.parent.close_pop_up() - def click(self, btn, value): + def click(self, value, btn): self.parent._action(value) self.parent.close_pop_up() diff --git a/subiquitycore/ui/selector.py b/subiquitycore/ui/selector.py index b464d3cf4..46fb3c638 100644 --- a/subiquitycore/ui/selector.py +++ b/subiquitycore/ui/selector.py @@ -60,7 +60,7 @@ def __init__(self, parent, cur_index): for i, option in enumerate(self.parent._options): if option.enabled: btn = ClickableThing(option.label) - connect_signal(btn, "click", self.click, i) + connect_signal(btn, "click", self.click, user_args=[i]) if i == cur_index: rhs = "\N{BLACK LEFT-POINTING SMALL TRIANGLE} " else: @@ -85,7 +85,7 @@ def __init__(self, parent, cur_index): list_box.base_widget.focus_position = cur_index super().__init__(Color.body(LineBox(list_box))) - def click(self, btn, index): + def click(self, index, btn): self.parent.index = index self.parent.close_pop_up() diff --git a/subiquitycore/ui/views/network.py b/subiquitycore/ui/views/network.py index 4a3d166cc..ecf497c64 100644 --- a/subiquitycore/ui/views/network.py +++ b/subiquitycore/ui/views/network.py @@ -88,7 +88,7 @@ def _create(self): actions.append((action.str(), True, (action, meth), opens_dialog)) menu = ActionMenu(actions) - connect_signal(menu, "action", self.parent._action, self) + connect_signal(menu, "action", self.parent._action, user_args=[self]) trows = [ make_action_menu_row( @@ -344,7 +344,7 @@ def _action_DELETE(self, name, dev_info): self.controller.delete_link(dev_info.name) self.del_link(dev_info) - def _action(self, sender, action, netdev_table): + def _action(self, netdev_table, sender, action): action, meth = action dev_info = netdev_table.dev_info meth("{}/{}".format(dev_info.name, action.name), dev_info) From d9005e0eb3687e2072d6cc1ebfcd786cd082c7ed Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 27 Aug 2024 10:12:16 +0200 Subject: [PATCH 2/4] ui: use AttrMap instead of AttrWrap urwid deprecated AttrWrap in favor of AttrMap. It is pretty easy to replace one with the other and is backward compatible with urwid from core22. Signed-off-by: Olivier Gayot --- subiquitycore/ui/actionmenu.py | 4 ++-- subiquitycore/ui/selector.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/subiquitycore/ui/actionmenu.py b/subiquitycore/ui/actionmenu.py index 81accb95b..cc0e45a34 100644 --- a/subiquitycore/ui/actionmenu.py +++ b/subiquitycore/ui/actionmenu.py @@ -15,7 +15,7 @@ import attr from urwid import ( ACTIVATE, - AttrWrap, + AttrMap, Button, LineBox, PopUpLauncher, @@ -83,7 +83,7 @@ def __init__(self, parent): ], dividechars=1, ) - btn = AttrWrap(btn, "info_minor") + btn = AttrMap(btn, "info_minor") group.append(btn) self.width = width super().__init__(Color.body(LineBox(ListBox(group)))) diff --git a/subiquitycore/ui/selector.py b/subiquitycore/ui/selector.py index 46fb3c638..0dae1bda9 100644 --- a/subiquitycore/ui/selector.py +++ b/subiquitycore/ui/selector.py @@ -12,7 +12,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from urwid import ACTIVATE, AttrWrap, CompositeCanvas, LineBox +from urwid import ACTIVATE, AttrMap, CompositeCanvas, LineBox from urwid import Padding as UrwidPadding from urwid import PopUpLauncher, Text, connect_signal @@ -76,9 +76,9 @@ def __init__(self, parent, cur_index): ] ) if option.enabled: - row = AttrWrap(row, "menu_button", "menu_button focus") + row = AttrMap(row, "menu_button", "menu_button focus") else: - row = AttrWrap(row, "info_minor") + row = AttrMap(row, "info_minor") btn = UrwidPadding(row, width=self.parent._padding.width) group.append(btn) list_box = ListBox(group) @@ -162,7 +162,7 @@ class Selector(WidgetWrap): def __init__(self, opts, index=0): self._icon = ClickableThing(Text("")) self._padding = UrwidPadding( - AttrWrap( + AttrMap( Columns( [ (1, Text("[")), From 337f7846f4336d99e321f408235e539a9aaf18b9 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 27 Aug 2024 10:32:06 +0200 Subject: [PATCH 3/4] ui: replace use of urwid's deprecated set_focus urwid from core24 raises deprecation notices when using set_focus on widgets. In core24, one can use the `container.focus = widget` setter property or `container.focus_position = index` setter property. However, in core22, the `container.focus` property is read-only. `container.focus_position` is available in core22 and core24 and does not produce deprecation warnings ; so let's use it. Signed-off-by: Olivier Gayot --- subiquity/ui/views/installprogress.py | 2 +- subiquitycore/ui/container.py | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/subiquity/ui/views/installprogress.py b/subiquity/ui/views/installprogress.py index 23f0b316c..4a16f81f6 100644 --- a/subiquity/ui/views/installprogress.py +++ b/subiquity/ui/views/installprogress.py @@ -84,7 +84,7 @@ def _add_line(self, lb, line): at_end = len(walker) == 0 or lb.focus_position == len(walker) - 1 walker.append(line) if at_end: - lb.set_focus(len(walker) - 1) + lb.focus_position = len(walker) - 1 lb.set_focus_valign("bottom") def event_start(self, context_id, context_parent_id, message): diff --git a/subiquitycore/ui/container.py b/subiquitycore/ui/container.py index 6ec18083c..a34572129 100644 --- a/subiquitycore/ui/container.py +++ b/subiquitycore/ui/container.py @@ -131,7 +131,7 @@ def _select_first_selectable(self): """Select first selectable child (possibily recursively).""" for i, (w, o) in enumerate(self.contents): if w.selectable(): - self.set_focus(i) + self.focus_position = i _maybe_call(w, "_select_first_selectable") return @@ -139,7 +139,7 @@ def _select_last_selectable(self): """Select last selectable child (possibily recursively).""" for i, (w, o) in reversed(list(enumerate(self.contents))): if w.selectable(): - self.set_focus(i) + self.focus_position = i _maybe_call(w, "_select_last_selectable") return @@ -189,7 +189,7 @@ def keypress(self, size, key): next_fp = self.focus_position + 1 for i, (w, o) in enumerate(self._contents[next_fp:], next_fp): if w.selectable(): - self.set_focus(i) + self.focus_position = i _maybe_call(w, "_select_first_selectable") return if not key.endswith(" no wrap"): @@ -199,7 +199,7 @@ def keypress(self, size, key): positions = self._contents[: self.focus_position] for i, (w, o) in reversed(list(enumerate(positions))): if w.selectable(): - self.set_focus(i) + self.focus_position = i _maybe_call(w, "_select_last_selectable") return if not key.endswith(" no wrap"): @@ -265,7 +265,7 @@ def _select_first_selectable(self): """Select first selectable child (possibily recursively).""" for i, (w, o) in enumerate(self.contents): if w.selectable(): - self.set_focus(i) + self.focus_position = i _maybe_call(w, "_select_first_selectable") return @@ -273,7 +273,7 @@ def _select_last_selectable(self): """Select last selectable child (possibily recursively).""" for i, (w, o) in reversed(list(enumerate(self.contents))): if w.selectable(): - self.set_focus(i) + self.focus_position = i _maybe_call(w, "_select_last_selectable") return @@ -289,14 +289,14 @@ def __init__(self, body): super().__init__(body) def _set_focus_no_move(self, i): - # We call set_focus twice because otherwise the listbox - # attempts to do the minimal amount of scrolling required to + # We set the focus_position property twice because otherwise the + # listbox attempts to do the minimal amount of scrolling required to # get the new focus widget into view, which is not what we # want, as if our first widget is a compound widget it results # its last widget being focused -- in fact the opposite of # what we want! - self.set_focus(i) - self.set_focus(i) + self.focus_position = i + self.focus_position = i # I don't really understand why this is required but it seems it is. self._invalidate() @@ -334,7 +334,7 @@ def keypress(self, size, key): next_fp = self.focus_position + 1 for i, w in enumerate(self.body[next_fp:], next_fp): if w.selectable(): - self.set_focus(i) + self.focus_position = i _maybe_call(w, "_select_first_selectable") return None if not key.endswith(" no wrap"): @@ -344,7 +344,7 @@ def keypress(self, size, key): positions = self.body[: self.focus_position] for i, w in reversed(list(enumerate(positions))): if w.selectable(): - self.set_focus(i) + self.focus_position = i _maybe_call(w, "_select_last_selectable") return None if not key.endswith(" no wrap"): From 38315aded14ddf28cb951e3a4f0cfa440494250f Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 27 Aug 2024 13:13:09 +0200 Subject: [PATCH 4/4] ui: use ListBox.focus instead accessing the ListWalker object Although ListBox.get_focus() is deprecated, using ListWalker.get_focus() through ListBox.body.get_focus() is not. However, the ListBox.focus property exists and makes it easy to access the underlying focused widget ; so use it instead of dereferencing the ListWalker object. Signed-off-by: Olivier Gayot --- subiquitycore/ui/container.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/subiquitycore/ui/container.py b/subiquitycore/ui/container.py index a34572129..1e5b03357 100644 --- a/subiquitycore/ui/container.py +++ b/subiquitycore/ui/container.py @@ -465,12 +465,11 @@ def render(self, size, focus=False): seen_focus = False height = height_before_focus = 0 - focus_widget, focus_pos = lb.body.get_focus() # Scan through the rows calculating total height and the # height of the rows before the focus widget. for widget in lb.body: rows = widget.rows((maxcol - 1,)) - if widget is focus_widget: + if widget is lb.focus: seen_focus = True elif not seen_focus: height_before_focus += rows