-
Notifications
You must be signed in to change notification settings - Fork 0
DTS changes #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
DTS changes #1
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,17 +3,16 @@ | |
| # Requires: yq (YAML processor) | ||
|
|
||
| # ANSI color codes (matching DTS color scheme) | ||
| readonly TUI_NORMAL='\033[0m' | ||
| readonly TUI_RED='\033[0;31m' | ||
| readonly TUI_GREEN='\033[0;32m' | ||
| readonly TUI_YELLOW='\033[0;33m' | ||
| readonly TUI_BLUE='\033[0;36m' # Cyan, used for borders (matches DTS BLUE) | ||
| TUI_NORMAL='\033[0m' | ||
| TUI_RED='\033[0;31m' | ||
| TUI_GREEN='\033[0;32m' | ||
| TUI_YELLOW='\033[0;33m' | ||
| TUI_BLUE='\033[0;36m' # Cyan, used for borders (matches DTS BLUE) | ||
|
|
||
| # Terminal width configuration | ||
| readonly TUI_MAX_WIDTH=60 # Maximum width for borders and footer wrapping | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why have you deleted |
||
| TUI_MAX_WIDTH=60 # Maximum width for borders and footer wrapping | ||
|
|
||
| # Global variables | ||
| TUI_CONFIG_FILE="" | ||
| TUI_RUNNING=true | ||
|
|
||
| # Header variables | ||
|
|
@@ -22,12 +21,18 @@ TUI_HEADER_SUBTITLE="" | |
| TUI_HEADER_LINK="" | ||
|
|
||
| # Section arrays (using | as delimiter) | ||
| declare -a TUI_SECTIONS_DATA=() # condition|label | ||
| declare -a TUI_ENTRIES_DATA=() # section_idx|condition|label|value | ||
| declare -a TUI_SECTIONS_DATA=() # condition|label | ||
| declare -a TUI_ENTRIES_DATA=() # section_idx|condition|label|value | ||
|
|
||
| # Menu and footer arrays | ||
| declare -a TUI_MENU_DATA=() # key|condition|label|callback | ||
| declare -a TUI_FOOTER_DATA=() # key|condition|label|callback | ||
| declare -a TUI_MENU_DATA=() # key|condition|label|callback | ||
| declare -a TUI_FOOTER_DATA=() # key|condition|label|callback | ||
|
|
||
| declare -A TUI_PRE_RENDER_CALLBACKS=() | ||
| declare -A TUI_POST_RENDER_CALLBACKS=() | ||
| # used to call callbacks in the same order as they were registered | ||
| TUI_PRE_RENDER_CALLBACKS_ORDER=() | ||
| TUI_POST_RENDER_CALLBACKS_ORDER=() | ||
|
Comment on lines
+31
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initializing arrays with empty values two times, why?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mistake There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It wasn't a mistake, those are different arrays
|
||
|
|
||
| # Terminal control | ||
| tui_clear_screen() { | ||
|
|
@@ -42,6 +47,10 @@ tui_show_cursor() { | |
| printf '\033[?25h' | ||
| } | ||
|
|
||
| tui_clear_line() { | ||
| printf '\r\033[K' | ||
| } | ||
|
|
||
| # Trap to ensure cursor is shown on exit | ||
| trap 'tui_show_cursor' EXIT INT TERM | ||
|
|
||
|
|
@@ -185,7 +194,7 @@ tui_expand_vars() { | |
| # Usage: tui_check_condition "condition" | ||
| tui_check_condition() { | ||
| local condition="$1" | ||
| [[ -z "$condition" ]] && return 0 # No condition means always show | ||
| [[ -z "$condition" ]] && return 0 # No condition means always show | ||
|
|
||
| # Expand and evaluate the condition | ||
| local result | ||
|
|
@@ -216,8 +225,6 @@ tui_load_config() { | |
| return 1 | ||
| fi | ||
|
|
||
| TUI_CONFIG_FILE="$config_file" | ||
|
|
||
| # Convert YAML to JSON once | ||
| local json_config | ||
| json_config=$(yq eval -o=json "$config_file") | ||
|
|
@@ -296,7 +303,7 @@ tui_render_info_sections() { | |
| local section_data | ||
| for section_data in "${TUI_SECTIONS_DATA[@]}"; do | ||
| local condition label | ||
| IFS='|' read -r condition label <<< "$section_data" | ||
| IFS='|' read -r condition label <<<"$section_data" | ||
|
|
||
| # Check if section should be displayed | ||
| if ! tui_check_condition "$condition"; then | ||
|
|
@@ -311,7 +318,7 @@ tui_render_info_sections() { | |
| local entry_data | ||
| for entry_data in "${TUI_ENTRIES_DATA[@]}"; do | ||
| local entry_section_idx entry_condition entry_label entry_value | ||
| IFS='|' read -r entry_section_idx entry_condition entry_label entry_value <<< "$entry_data" | ||
| IFS='|' read -r entry_section_idx entry_condition entry_label entry_value <<<"$entry_data" | ||
|
|
||
| # Only render entries for this section | ||
| if [[ "$entry_section_idx" != "$section_idx" ]]; then | ||
|
|
@@ -341,7 +348,7 @@ tui_render_menu() { | |
| local menu_item | ||
| for menu_item in "${TUI_MENU_DATA[@]}"; do | ||
| local key condition label callback | ||
| IFS='|' read -r key condition label callback <<< "$menu_item" | ||
| IFS='|' read -r key condition label callback <<<"$menu_item" | ||
|
|
||
| if ! tui_check_condition "$condition"; then | ||
| continue | ||
|
|
@@ -365,7 +372,7 @@ tui_render_footer() { | |
| # Build footer parts | ||
| for footer_item in "${TUI_FOOTER_DATA[@]}"; do | ||
| local key condition label callback | ||
| IFS='|' read -r key condition label callback <<< "$footer_item" | ||
| IFS='|' read -r key condition label callback <<<"$footer_item" | ||
|
|
||
| if ! tui_check_condition "$condition"; then | ||
| continue | ||
|
|
@@ -379,7 +386,7 @@ tui_render_footer() { | |
| # Auto-wrap footer to multiple lines if needed | ||
| local current_line="" | ||
| local current_length=0 | ||
| local max_width=$((TUI_MAX_WIDTH - 2)) # Leave 2 chars margin | ||
| local max_width=$((TUI_MAX_WIDTH - 2)) # Leave 2 chars margin | ||
|
|
||
| for part in "${footer_parts[@]}"; do | ||
| # Calculate visible length (strip ANSI codes) | ||
|
|
@@ -390,7 +397,7 @@ tui_render_footer() { | |
| # Add separator length if not first item on line | ||
| local separator_length=0 | ||
| if [[ -n "$current_line" ]]; then | ||
| separator_length=2 # " " = 2 spaces | ||
| separator_length=2 # " " = 2 spaces | ||
| fi | ||
|
|
||
| # Check if adding this part would exceed max width | ||
|
|
@@ -437,9 +444,31 @@ tui_read_key() { | |
| echo "$key" | ||
| } | ||
|
|
||
| # Print prompt and read user input | ||
| tui_read_prompt() { | ||
| local prompt="$1" | ||
| local answer | ||
| echo -n "${prompt}: " >&2 | ||
| read -r answer | ||
| echo "${answer}" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So it can be saved to variable (echoed to stdout). User facing output is outputted to |
||
| } | ||
|
|
||
| tui_read_key_to_continue() { | ||
| # Wait for user to press a key before returning to menu | ||
| echo "" >&2 | ||
| echo -n "Press any key to continue..." >&2 | ||
| tui_read_key | ||
| } | ||
|
|
||
| tui_read_enter_to_continue() { | ||
| # Wait for user to press enter before returning to menu | ||
| echo "" >&2 | ||
| echo -n "Press Enter to continue..." >&2 | ||
| read -r &>/dev/null | ||
| } | ||
|
|
||
| # Execute a callback script | ||
| # Usage: tui_execute_callback "script_path" | ||
| tui_execute_callback() { | ||
| tui_execute_callback_without_waiting() { | ||
| local script="$1" | ||
|
|
||
| if [[ ! -f "$script" ]]; then | ||
|
|
@@ -457,13 +486,15 @@ tui_execute_callback() { | |
|
|
||
| # Execute the callback script | ||
| "$script" | ||
| local exit_code=$? | ||
| } | ||
|
|
||
| # Wait for user to press a key before returning to menu | ||
| echo "" | ||
| echo -n "Press any key to continue..." | ||
| tui_read_key | ||
| # Execute a callback script and wait for user input | ||
| # Usage: tui_execute_callback "script_path" | ||
| tui_execute_callback() { | ||
| tui_execute_callback_without_waiting "$@" | ||
| local exit_code=$? | ||
|
|
||
| tui_read_enter_to_continue | ||
| return $exit_code | ||
| } | ||
|
|
||
|
|
@@ -473,7 +504,7 @@ tui_find_menu_callback() { | |
| local key="$1" | ||
|
|
||
| for menu_item in "${TUI_MENU_DATA[@]}"; do | ||
| IFS='|' read -r menu_key condition label callback <<< "$menu_item" | ||
| IFS='|' read -r menu_key condition label callback <<<"$menu_item" | ||
|
|
||
| if [[ "$menu_key" == "$key" ]]; then | ||
| if tui_check_condition "$condition"; then | ||
|
|
@@ -494,7 +525,7 @@ tui_find_footer_callback() { | |
| local key="$1" | ||
|
|
||
| for footer_item in "${TUI_FOOTER_DATA[@]}"; do | ||
| IFS='|' read -r footer_key condition label callback <<< "$footer_item" | ||
| IFS='|' read -r footer_key condition label callback <<<"$footer_item" | ||
|
|
||
| if [[ "$footer_key" == "$key" ]]; then | ||
| if tui_check_condition "$condition"; then | ||
|
|
@@ -533,7 +564,7 @@ tui_handle_input() { | |
|
|
||
| # Try to find callback in footer items | ||
| if callback=$(tui_find_footer_callback "$key"); then | ||
| tui_execute_callback "$callback" | ||
| tui_execute_callback_without_waiting "$callback" | ||
| return 0 | ||
| fi | ||
|
|
||
|
|
@@ -554,7 +585,13 @@ tui_run() { | |
| TUI_RUNNING=true | ||
|
|
||
| while $TUI_RUNNING; do | ||
| for callback in "${TUI_PRE_RENDER_CALLBACKS_ORDER[@]}"; do | ||
| eval "${callback}" "${TUI_PRE_RENDER_CALLBACKS["${callback}"]}" | ||
| done | ||
| tui_render | ||
| for callback in "${TUI_POST_RENDER_CALLBACKS_ORDER[@]}"; do | ||
| eval "${callback}" "${TUI_POST_RENDER_CALLBACKS["${callback}"]}" | ||
| done | ||
| tui_handle_input | ||
| done | ||
|
|
||
|
|
@@ -567,6 +604,23 @@ tui_stop() { | |
| TUI_RUNNING=false | ||
| } | ||
|
|
||
| # register callbacks called during each UI refresh (before showing UI) e.g. | ||
| # tui_register_refresh_callback my_callback_func callback_arg_1 callback_arg_2 | ||
| tui_register_pre_render_callback() { | ||
| local callback="$1" | ||
| shift | ||
| TUI_PRE_RENDER_CALLBACKS["${callback}"]="$*" | ||
| TUI_PRE_RENDER_CALLBACKS_ORDER+=("${callback}") | ||
| } | ||
|
|
||
| # register callbacks called during each UI refresh (after showing UI) | ||
| tui_register_post_render_callback() { | ||
| local callback="$1" | ||
| shift | ||
| TUI_POST_RENDER_CALLBACKS["${callback}"]="$*" | ||
| TUI_POST_RENDER_CALLBACKS_ORDER+=("${callback}") | ||
| } | ||
|
|
||
| # Export functions for use in other scripts | ||
|
|
||
| # Terminal control | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # Notes | ||
|
|
||
| * `TUI_CONFIG_FILE` in `tui-lib.sh` is unused | ||
| * We are loading YAML config with `yq`, converting it to JSON and then using | ||
| `jq` to parse it. Use YAML directly. | ||
| * Related to previous point, functions for parsing this config are unreadable | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Parsing YAML should be easy to change and does not impact the general architecture. |
||
| * Callbacks have to be full path to file, can't use commands | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like we are gathering some requirements
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we consider this a requirement to be able to use either command or script as a callback? Or maybe also a function? Or maybe having too much flexibility and options would lead us to convoluted solutions, and executing each action as another script (even if it uses only one comand) is fine? Or we should decide on one of these two:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would consider using arguments for the commands in the config files as well. |
||
| * Why are we exporting functions at least ones that likely shouldn't be used by | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once we have clear expectations on what is used and how, it can/should be limited. |
||
| backend like `tui_stop` | ||
| * We should split `tui-lib.sh` into lib that should be used by scripts to print | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking of the same. |
||
| something, and core lib used to run menu. | ||
| * Add input parsing in menu to disallow multiline strings (or deal with them | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you give an example here?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. diff --git a/examples/demo.sh b/examples/demo.sh
index 1920b5d429b6..3ecf767a8d3a 100755
--- a/examples/demo.sh
+++ b/examples/demo.sh
@@ -15,3 +15,6 @@ export BASEBOARD_INFO="Emulation QEMU x86 q35/ich9"
export CPU_INFO="Intel Core Processor (Skylake)"
-export RAM_INFO="Not Specified"
+export RAM_INFO="RAM1: 2 GB
+RAM2: 2GB
+RAM3: None
+RAM4: None"
export BIOS_INFO="3mdeb Dasharo (coreboot+UEFI) v0.2.1-rc1"Same with strings that are too long, they'll either move outside of border or wrap around |
||
| correctly) | ||
| * Add right border (and handle too long strings) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You mean
Handle how? Wrap? Prevent? |
||
| * callback can't have arguments | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we probably need option 2 from: https://github.com/3mdeb/tui-sh/pull/1/files#r2471258244 and also arguments handling? |
||
| * conditions can only be variables | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really want to put more complex shell expressions in the YAML? Can you give some examples?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return value of a command |
||
| * `tui_handle_input` | ||
| - `# Hidden option: Q to quit (useful for testing)` - I am very apprehensive | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was on my request to quickly shutdown when testing demo and have the same options as current DTS. Can easily be dropped. |
||
| about `hidden` part. | ||
| - I thought this function was to be used by other scripts, but it shouldn't | ||
| as this function is to handle menu input. We really need to separate core | ||
| functions, internal to the UI. | ||
| * Missing various input, prompt, choice, etc. functions | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was not the point of this lib at this point. The point was main menu drawing. These functions could be added to this second library. Do you have a complete list of the input/prompt/choice primitives we would need based on your integration experiment? |
||
| * We can only print lines (with newline at the end), if we want to print | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we phrase it differently: what function is missing, or what kind of change is needed here? |
||
| multicolor line we have to use workarounds such as: | ||
|
|
||
| ```sh | ||
| tui_echo_normal "$(tui_echo_red ERROR:) $(tui_echo_yellow warning)" | ||
| ``` | ||
|
|
||
| we should have clear split between functions that should only be used to | ||
| print on screen and functions that should be used to e.g. pass strings | ||
| between functions, variables etc. In the future, we could make sure that | ||
| we only show text that is printed via those screen printing functions | ||
| * Find better way to do submenus - in DTS menu was generated dynamically so I | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any ideas here? Does it mean static YAML is not well fit for us? Or maybe the menu creation logic in DTS is overcomplicated?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To enter submenu we have to create separate script that will call |
||
| created temporary `yaml` and ran `tui_run <path_to_new_config>"` | ||
| * `Enter an option:|` cursor is too close (add space) `Enter an option: |` | ||
| * Allow using commands instead of shell variables? | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is the same as: https://github.com/3mdeb/tui-sh/pull/1/files#r2471267205 I guess
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that was related to information sections. E.g. something like: https://github.com/iwanicki92/tui-test/blob/ae7759d64196d01f0e7ee9f3a97630eca60daaf3/config.yml#L18 We call the function, format the output (e.g. get only first line, trim if too long, e.t.c.) and print on screen |
||
| * Missing utility functions related to e.g. asking for user choice e.g.: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same as: https://github.com/3mdeb/tui-sh/pull/1/files#r2471269337 ? But I appreciate how much more concrete this one is. |
||
| <https://github.com/Dasharo/dts-scripts/blob/7b43513360816fc2171161b39c2a4bc79f88f487/include/dts-functions.sh#L1921> | ||
| * Feature: we could force usage of TUI printing functions to print anything on | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as 10 lines above? |
||
| screen. Usage of `echo` wouldn't print anything, and could be used to pass | ||
| strings between functions in shell script. | ||
| * DTS related: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to convert these into any specific tasks to be done on the DTS side? |
||
| - delay before refresh after pressing any, non-mapped key (likely due to | ||
| `subscription_routine` or other pre-render callbacks) | ||
| - long black screen when using footer options, probably the same reason as | ||
| before, screen is cleared, `pre-render` callbacks are running and only | ||
| after that we render UI | ||
| - Extensions in DTS extensions submenu might not work | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why have you deleted
readonly?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warnings when sourcing this file multiple times