Print install and remove messages below transaction summary #421
Conversation
There was a problem hiding this comment.
I think an important part missing here is an option to disable printing the info. I think having messages below the summary helps with discoverability, at least, but maybe we should consider printing a message to look above the messages for the summary, if any message was printed? Might help unaware users.
Unrelated: these install msgs should all be removed.
bin/xbps-install/transaction.c
Outdated
| const char *pkgname = xbps_string_cstring_nocopy(xbps_dictionary_get(obj, "pkgname")); | ||
| xbps_dictionary_t pkgdb_pkg = pkgname ? xbps_pkgdb_get_pkg(xhp, pkgname) : NULL; | ||
| xbps_object_t previous = pkgdb_pkg ? xbps_dictionary_get(pkgdb_pkg, "install-msg") : NULL; | ||
| xbps_cb_message(xhp, obj, "install-msg", previous); | ||
| break; |
There was a problem hiding this comment.
Can you put a comment explaining this? I don't get the logic.
There was a problem hiding this comment.
Here install-msg of installed version of package is read from system pkgdb, and passed to xbps_cb_message.
Then xbps_cb_message don't print message if current and previous are same.
Is doc comment of xbps_cb_message best place to write it down?
There was a problem hiding this comment.
Then xbps_cb_message don't print message if current and previous are same.
Not sure how I feel about this... If someone misses an INSTALL.msg, they will never see it again.
Is doc comment of xbps_cb_message best place to write it down?
Yeah, I guess so. And comment this code block "get installed version of package, if any".
bin/xbps-remove/main.c
Outdated
| if (chars_print < sizeof bar) { | ||
| printf("%s", bar + chars_print); | ||
| } | ||
| printf("\n%s", xscd->desc); |
There was a problem hiding this comment.
if (chars_print < sizeof bar) {
puts(bar + chars_print);
}
fputs(xscd->desc, stdout); // not sure if you like this one, otherwise keep the printf
There was a problem hiding this comment.
important part missing here is an option to disable printing the info
Why? I feel something is done wrong if this needs a toggle.
maybe we should consider printing a message to look above
May be helpful when message take all screen, indeed.
Changed fprint to fputs.
Messages during transaction are still printed, and never skipped, but this can be changed to no more print during transaction, I think.
bin/xbps-install/transaction.c
Outdated
| const char *pkgname = xbps_string_cstring_nocopy(xbps_dictionary_get(obj, "pkgname")); | ||
| xbps_dictionary_t pkgdb_pkg = pkgname ? xbps_pkgdb_get_pkg(xhp, pkgname) : NULL; | ||
| xbps_object_t previous = pkgdb_pkg ? xbps_dictionary_get(pkgdb_pkg, "install-msg") : NULL; | ||
| xbps_cb_message(xhp, obj, "install-msg", previous); | ||
| break; |
There was a problem hiding this comment.
Here install-msg of installed version of package is read from system pkgdb, and passed to xbps_cb_message.
Then xbps_cb_message don't print message if current and previous are same.
Is doc comment of xbps_cb_message best place to write it down?
You could have read them already and not want the spam, or to pipe the output through some filter/program (even if it's not advised), or something else I can't think of. |
|
And I don't see why keep the messages during installation. The most I'd do is print them after as a block reminder (but maybe only for the packages that succeeded, if a hook fails or something - assuming that's easy to implement). |
|
I'm not convinced about switch to disable. Config file don't fit, because you don't want to disable and forget. Flag don't fit, because existing tools won't use it, and adapted could use
This is solved by showing message just once, administrators of multiple systems are encouraged to skim over anyway.
removed Mentioning packages list is added, but not really nice. |
Chocimier
force-pushed
the
install-msg
branch
from
00e6cac
to
89c32af
Compare
|
Rebased, removed last line pointing to packages list and replaced static linking with exposing function, as agreed in other PR. |
|
I don't think breaking the |
Makes messages actually visible, and allow to delay update in case manual steps required after update are inconvenient at given time.
As is, presence of messages breaks tools parsing xbps-install -n output, but I want that despite.
There are no install messages in xbps or octoxbps dependency tree, so system update won't be affected.
Pushed on top of static linking as publishing neitherxbps_cb_messagenorshow_package_msgsused in xbps-install make sense.Also changed formatting a bit, now it looks like: