-
-
Notifications
You must be signed in to change notification settings - Fork 831
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
feat(core): prefix logs with a timestamp #3795
feat(core): prefix logs with a timestamp #3795
Conversation
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.
the standard on lgsm is more "on/off" and not "True/False"
Please refactor this one first
Great idea!! |
You think it needs anything else to be merged? |
lgsm/functions/command_start.sh
Outdated
@@ -8,6 +8,7 @@ | |||
commandname="START" | |||
commandaction="Starting" | |||
functionselfname="$(basename "$(readlink -f "${BASH_SOURCE[0]}")")" | |||
add_ts="gawk '{ print strftime(\\\"[$logtsformat]\\\"), \\\$0 }'" |
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.
Suggesting to use date instead of gawk:
printf '[%s] %s\n' "$(date "+$logtsformat")" "hello world"
or echo "asd" | xargs printf '[%s] %s\n' "$(date "+$format")"
provides the same output as
echo "hello world" | gawk '{ print strftime("[%Y-%m-%d %H:%M:%S]"), $0}'
and we wouldnt need an additional dependency
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.
I had a hard time escaping that command to be able to use it in my exec . If you managed to do it I'd be happy to use it.
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.
Simple explanation:
bash knows mostly strings,
"interpret special characters >once< before passing this to something"
'dont interpret special characters before passing'
Therefore:
echo "$UID" '$UID' # whitespace in between = two strings = two args for echo
1st is checked for special characters and after that added as arg to echo, 2nd not
If you dont want this behavior use single quotes.
If you need to mix it, you can combine it.
echo "$UID"'$UID' # no whitespace in between = one string, first part is interpreted before
or you can escape with
echo "\$UID" '$UID' # output is identical
if you do want to output \ between "", you need to escape it as well:
echo "\\" '\' # but not if you use single quotes
What if you want to use double or single quotes?
echo "\"" '"'
or echo "'"
You can also escape whitespaces, so this is one string
foo=hello\ world
but please dont do so
In your command: add_ts="gawk '{ print strftime(\\\"[$logtsformat]\\\"), \\\$0 }'"
add_ts=".."..".."
you used the "interpret special characters way" but you wanted to add double quotes to it, so you needed to escape it once. add_ts="..\"..\".."
but why are the additional \ needed?
foo=hello
bar=\$foo
echo "$bar" # prints $foo because interpreted once, even if special symbol is in
eval echo "$bar" # prints hello but why?
# calls eval "echo" '$foo'
# eval interpreted once more and executes "echo" "hello"
#
# if you want to prevent it ( and how you did )
bar=\\\$foo
eval echo "$bar" # prints now $foo instead of hello
# thats what happened to you
Edit btw if you want to execute a command try:
cmd=(echo "hello world")
cmd+=("123")
"${cmd[@]}"
# | & > aso need to be outside of the array
"${cmd[@]}" | ...
Sry was more motivated to clarify this for you than fixing the issue.
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.
I understand all that, but I need it to process the variable $logtsformat
inside of add_ts
. Which means I have:
add_ts=gawk '{ print strftime(\"[%Y-%m-%d %H:%M:%S]\"), \$0 }'
Then in L96 the line will become:
tmux pipe-pane -o -t "${sessionname}" "exec bash -c \"cat | gawk '{ print strftime(\"[%Y-%m-%d %H:%M:%S]\"), \$0 }'\" >> '${consolelog}'"
I can put everything manually in that line, but we'll have the some logic repeated in multiple steps and it'll make the code less readable and harder to maintain.
f02e53d
to
2257be4
Compare
gawk is a standard tool on linux distros and no check is required
* feat: Added Option to prefix logs with timestamps * feat: Added Option to prefix logs with timestamps * Add parameters to every game * feat: Switched timestamp enabling from True/False to on/off * feat: Added quotes on variables * add to new configs * renamed to logtimestamp * remove gawk as a requirement gawk is a standard tool on linux distros and no check is required * tidy * rename to addtimestamp --------- Co-authored-by: Reda.drissi.e <[email protected]> Co-authored-by: Daniel Gibbs <[email protected]>
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Adds the possibility to prefix each line of log with a timestamp before pushing it to the log file.
Currently I only enabled it on rustserver, because that's what I used to test it. But it should work for everything else.
I pushed the modification to every file, but if #3745 is merged, I'll refactor my PR.
Fixes #3778
Type of change
Checklist
PR will not be merged until all steps are complete.
develop
branch as its base.