Skip to content

Color-code install preview: red for errors, yellow for warnings, green for ready#4512

Open
Softer wants to merge 11 commits into
archlinux:masterfrom
Softer:colored-install-preview
Open

Color-code install preview: red for errors, yellow for warnings, green for ready#4512
Softer wants to merge 11 commits into
archlinux:masterfrom
Softer:colored-install-preview

Conversation

@Softer
Copy link
Copy Markdown
Contributor

@Softer Softer commented May 1, 2026

Summary

  • Add preview_markup opt-in field to MenuItem - when True, the preview panel renders Rich markup; when False (default), text is auto-escaped so existing previews are unaffected
  • Move network warning from the confirmation dialog to the install preview, where the user sees it earlier - while still configuring, not after pressing Install
  • All other preview panels (profile, disk, locale, etc.) are unchanged - preview_markup=False by default escapes any [...] sequences in their output

Color codes

  • Red: missing mandatory configs, bootloader validation errors (blocking)
  • Yellow: warnings like "no network configuration selected" (non-blocking)
  • Green: "Ready to install"

Examples

image image image image

Test plan

  • Open archinstall, leave disk config empty - Install preview should show red "Missing configurations" list
  • Configure disk but skip network - Install preview should show green "Ready to install" + yellow "Warnings" with network message
  • Leave both disk and network unconfigured - should show red missing configs + yellow network warning together
  • Configure everything including network - green "Ready to install" + summary, no yellow
  • Check that other menu items (profile, locale, mirrors) still show plain-text previews without markup artifacts
  • Press Install, confirm dialog should no longer contain the network warning line

…n for ready

Add preview_markup opt-in field to MenuItem with automatic Rich markup
escaping for all existing previews. Show missing configs and bootloader
errors in red, network warning in yellow, "Ready to install" in green.
Move network warning from confirmation dialog to install preview so it
is visible earlier.
@Softer Softer requested a review from Torxed as a code owner May 1, 2026 22:29
@Softer Softer marked this pull request as draft May 2, 2026 01:35
@Softer
Copy link
Copy Markdown
Contributor Author

Softer commented May 2, 2026

Converted into draft because looks like I found an error. Will check a bit later...

Text.from_markup() replaces Label(markup=True) to avoid MarkupError
on strings containing ["
@Softer Softer marked this pull request as ready for review May 2, 2026 10:30
@Softer
Copy link
Copy Markdown
Contributor Author

Softer commented May 2, 2026

Done, now it works correctly :-)

Comment thread archinstall/tui/ui/menu_item.py Outdated
Comment thread archinstall/lib/global_menu.py Outdated
@Softer
Copy link
Copy Markdown
Contributor Author

Softer commented May 3, 2026

Renamed InputInfoType to MsgLevelType, added PreviewResult dataclass, removed preview_markup from MenuItem. The widget now maps level to color instead of business logic producing Rich markup. preview_action accepts str | PreviewResult | list[PreviewResult] | None so sections with different levels can be mixed in one preview.

…ture

Update imports from archinstall.tui.ui.* to archinstall.tui.* to match
the module reorganization in archlinux#4515 (Pull ui module one level up).
@svartkanin
Copy link
Copy Markdown
Collaborator

@Softer when you addressed the comments I will have another review

@Softer
Copy link
Copy Markdown
Contributor Author

Softer commented Jun 3, 2026

@svartkanin Both comments addressed in c5d5135.

Also merged master to resolve the conflict status.

@svartkanin
Copy link
Copy Markdown
Collaborator

@Softer my previous comments have not all been addressed

@Softer
Copy link
Copy Markdown
Contributor Author

Softer commented Jun 6, 2026

@Softer my previous comments have not all been addressed

@svartkanin Could you clarify which comment you feel wasn't addressed?

Comment thread archinstall/lib/configuration.py Outdated
@@ -152,18 +149,10 @@ def get_install_warnings(self) -> list[str]:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be moved into the global_menu.py now as there's no purpose for this here anymore

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Softer this is still not addressed

errors += '\n'.join(f'- {m}' for m in missing)

disk_item = self._item_group.find_by_key('disk_config')
if disk_item.has_value():
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the same logic as the existing code, if there is a bug it'd be good to make these changes incremental to understand why they have changed

Comment thread archinstall/tui/menu_item.py Outdated
@@ -18,7 +33,7 @@ class MenuItem:
dependencies: list[str | Callable[[], bool]] = field(default_factory=list)
dependencies_not: list[str] = field(default_factory=list)
display_action: Callable[[Any], str] | None = None
preview_action: Callable[[Self], str | None] | None = None
preview_action: Callable[[Self], str | PreviewResult | list[PreviewResult] | None] | None = None
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be changed to only support the new wrapper type via

preview_action: Callable[[Self], PreviewResult | None] | None = None

The PreviewResult should then handle the different chunks or whatever a preview may return. This will have to be updated for all previews, so I recommend to do this in an incremental way with multiple PRs.

To make life easier we can have something like

@dataclass
class PreviewResult:
	messages: list[tuple[str, MsgLevelType]]

Comment thread archinstall/tui/components.py Outdated
from archinstall.tui.ui.result import Result, ResultType

ValueT = TypeVar('ValueT')


_LEVEL_STYLE: dict[MsgLevelType, str] = {
Copy link
Copy Markdown
Collaborator

@svartkanin svartkanin May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't create these global variables. This is a component of the type

class MsgLevelType(Enum):
	MsgNone = auto()
	MsgInfo = auto()
	MsgWarning = auto()
	MsgError = auto()

	def style(self) -> str:
		match self:
			case MsgLevelType.MsgInfo:
				return 'green'
            case MsgLevelType.MsgNone:
				return 'white'
			case MsgLevelType.MsgWarning:
				return 'yellow'
			case MsgLevelType.MsgError:
				return 'red'

@svartkanin
Copy link
Copy Markdown
Collaborator

@Softer apologies, I just realized that I never sent the comments and they all remained in the pending state.

Softer added 3 commits June 6, 2026 12:47
The method is only used by GlobalMenu._prev_install_invalid_config(),
so it belongs there rather than on the serialization class.
Replace the module-level _LEVEL_STYLE dict in components.py with a
style() method on the MsgLevelType enum, following the project
convention of encapsulating type-bound logic on the type itself.
PreviewResult.messages is now list[tuple[str, MsgLevelType]], allowing
a single result to carry multiple sections with different levels.
The preview_action signature drops list[PreviewResult] since the
dataclass itself handles multiple sections. Existing str-returning
previews still work and will be converted in follow-up PRs.
@Softer
Copy link
Copy Markdown
Contributor Author

Softer commented Jun 6, 2026

@svartkanin Addressed all comments:

  1. configuration.py - Moved get_install_warnings() to GlobalMenu._get_install_warnings() since it's the only caller (9c40f8e)
  2. components.py - Replaced _LEVEL_STYLE dict with MsgLevelType.style() method on the enum (139bd10)
  3. menu_item.py - Changed PreviewResult to messages: list[tuple[str, MsgLevelType]] so a single result holds multiple sections. Dropped list[PreviewResult] from preview_action signature. Existing str-returning previews still work, will convert them in a follow-up PR (a44edab)
  4. global_menu.py - The logic change in _prev_install_invalid_config is intentional: the original does early return on the first error, so the user only sees one problem at a time (missing configs hides the bootloader error). The new version collects all errors into one red section and always shows warnings alongside. Also added a guard: bootloader validation only runs when disk_config has a value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants