Skip to content

improve verbosity#283

Closed
LocutusOfBorg wants to merge 1 commit intodkms-project:masterfrom
LocutusOfBorg:messaging
Closed

improve verbosity#283
LocutusOfBorg wants to merge 1 commit intodkms-project:masterfrom
LocutusOfBorg:messaging

Conversation

@LocutusOfBorg
Copy link
Contributor

log_action_msg is a debianism

@scaronni
Copy link
Member

scaronni commented Dec 5, 2022

@LocutusOfBorg
Copy link
Contributor Author

which changes sorry?

@xuzhen
Copy link
Collaborator

xuzhen commented Feb 8, 2023

@evelikov
Copy link
Collaborator

evelikov commented Feb 8, 2023

Precisely ^^ - the log... is an alias for echo for non-Debian. If there is a better alternative do send a PR, but properly sanity check across distros.

@LocutusOfBorg
Copy link
Contributor Author

@evelikov @xuzhen @scaronni thank you! I admit I didn't know where that function came from in Debian.
I implemented a stub of the function for non-Debian systems, so we can use the same function everywhere.
What do you think? this approach looks cleaner to me

@evelikov
Copy link
Collaborator

evelikov commented Feb 9, 2023

Can you share and example of the changes this brings - a before/after picture would be nice. Also please squash the commits.

log_action_msg is a debianism

implement log_action_msg on non-Debian systems too
(patch also contributed by Andreas Beckmann <anbe@debian.org>)
@LocutusOfBorg
Copy link
Contributor Author

LocutusOfBorg commented Feb 10, 2023

    log_end_msg() { if [ "$1" = "0" ]; then echo " Done. "; else echo " Failed. "; fi; }
    # inspired from lsb init-functions script
    log_action_msg_pre () { :; }
    log_action_msg_post () { :; }
    log_action_msg () {
      log_action_msg_pre "$@"
      echo "$@." || true
       log_action_msg_post "$@"
    }


   touch /proc/foo
   log_action_msg "this is a failure"
   echo command that works
   log_action_msg "this one is a success"

prints a

touch: cannot touch '/proc/foo': No such file or directory
this is a failure.
command that works
this one is a success.

@LocutusOfBorg
Copy link
Contributor Author

LocutusOfBorg commented Feb 10, 2023

old version

    alias log_daemon_msg=/bin/echo
    log_end_msg() { if [ "$1" = "0" ]; then echo " Done. "; else echo " Failed. "; fi; }
    alias log_action_begin_msg=log_daemon_msg
    alias log_action_end_msg=log_end_msg

    touch /proc/foo
    log_daemon_msg "this is a failure"
    echo command that works
    log_daemon_msg "this one is a success"

prints:

touch: cannot touch '/proc/foo': No such file or directory
this is a failure
command that works
this one is a success

@LocutusOfBorg
Copy link
Contributor Author

For Debian, no changes in prints since we have the same way we were defined before, for non-debian systems the prints are now looking similar and the code is now shared

@LocutusOfBorg
Copy link
Contributor Author

Hello, ping?

@evelikov
Copy link
Collaborator

evelikov commented Mar 2, 2023

Sorry for the delay. This seems to add an extra full-stop, which does not seem beneficial - so personally I'll abstain.

The other two maintainers (commented above) are welcome to merge it if they choose to.

@evelikov evelikov mentioned this pull request Mar 21, 2023
@evelikov
Copy link
Collaborator

It seems that the issue wasn't about verbosity or Debian vs others. But a case of missing \n.

log_daemon_msg should not include the newline, since it's paired with log_end_msg which provides it. Relatedly log_action_msg should include the newline, since it'a standalone message.

Hats off to anbe42 for the background and fix with #315

Closing this PR, since it's superseded by the above.

@evelikov evelikov closed this Mar 23, 2023
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.

4 participants