| Message ID | 20180718213736.22650-1-semen.protsenko@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] env: Fix errors printing on env loading | expand |
Dear Sam, In message <20180718213736.22650-1-semen.protsenko@linaro.org> you wrote: > This is just a draft to discuss ideas related to "Make U-Boot log great > again" thread. > > With this patch we will have something like this: > > Attempting to load environment from FAT: > MMC: no card present > ** Bad device mmc 0 ** > Failed (-5) > Attempting to load environment from MMC: OK > > instead of this: > > Loading Environment from FAT... MMC: no card present > ** Bad device mmc 0 ** > Failed (-5) > Loading Environment from MMC... OK The new output is worse, as it consumes even more lines of output. > The only way I see to do so is to use ASCII escape codes for moving the > cursor (in non-error case). NAK!!! Please keep in mind that output usually goes to a serial port, and should not assume any speific "terminal" capabilities. Even simple in-line "editing" like "overprinting by outputting backspace characters causes a LOT of hassle when you try to parse output for example in automatic test suites. Adding more complex terminal control or other fancy stuff (like colors etc) is a _strict_ no, no! > I'd also like to add prefixes to error messages, like it's done in [2], > but it requires adding one pointer to global data struct. As I explained befoire, this has a lot of negative consequences. You think about beauty, but please keep functionality and efficiency (boot time) in mind! Best regards, Wolfgang Denk
diff --git a/env/env.c b/env/env.c index 5c0842ac07..a674ac2eab 100644 --- a/env/env.c +++ b/env/env.c @@ -187,6 +187,7 @@ int env_load(void) for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) { int ret; + char msg[75]; if (!drv->load) continue; @@ -194,12 +195,17 @@ int env_load(void) if (!env_has_inited(drv->location)) continue; - printf("Loading Environment from %s... ", drv->name); + snprintf(msg, 75, "Attempting to load environment from %s:\n", + drv->name); + puts(msg); ret = drv->load(); - if (ret) + if (ret) { printf("Failed (%d)\n", ret); - else - printf("OK\n"); + } else { + size_t len = strlen(msg); + + printf("\033[1A\033[%zuC OK\n", len - 1); + } if (!ret) return 0;
This is just a draft to discuss ideas related to "Make U-Boot log great again" thread. With this patch we will have something like this: Attempting to load environment from FAT: MMC: no card present ** Bad device mmc 0 ** Failed (-5) Attempting to load environment from MMC: OK instead of this: Loading Environment from FAT... MMC: no card present ** Bad device mmc 0 ** Failed (-5) Loading Environment from MMC... OK The only way I see to do so is to use ASCII escape codes for moving the cursor (in non-error case). I'd also like to add prefixes to error messages, like it's done in [2], but it requires adding one pointer to global data struct. [1] https://en.wikipedia.org/wiki/ANSI_escape_code#CSI_sequences [2] https://lists.denx.de/pipermail/u-boot/2018-July/335072.html Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> --- env/env.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)