Add timestamps to each message #82
Conversation
Enabled with the -t flag. Fixes JakeWharton#9
| @@ -41,6 +41,7 @@ | |||
| parser.add_argument('-c', '--clear', dest='clear_logcat', action='store_true', help='Clear the entire log before running.') | |||
| parser.add_argument('-t', '--tag', dest='tag', action='append', help='Filter output by specified tag(s)') | |||
| parser.add_argument('-i', '--ignore-tag', dest='ignored_tag', action='append', help='Filter output by ignoring specified tag(s)') | |||
| parser.add_argument('--timestamp', dest='add_timestamp', action='store_true', help='Prepend each line of output with the current time.') | |||
There was a problem hiding this comment.
What happened to the first argument ('-t')?
There was a problem hiding this comment.
Removed it because it's already being used to filter by tag.
|
Looks good to me other than my one comment! |
|
The output looks to be the same. My biggest gripe with the original PR was how it looked. For instance, the date is not useful information. |
|
@JakeWharton I've removed the date already. Is there anything else you'd like me to change? |
| @@ -145,7 +146,10 @@ def allocate_color(tag): | |||
| PID_KILL = re.compile(r'^Killing (\d+):([a-zA-Z0-9._:]+)/[^:]+: (.*)$') | |||
| PID_LEAVE = re.compile(r'^No longer want ([a-zA-Z0-9._:]+) \(pid (\d+)\): .*$') | |||
| PID_DEATH = re.compile(r'^Process ([a-zA-Z0-9._:]+) \(pid (\d+)\) has died.?$') | |||
| LOG_LINE = re.compile(r'^([A-Z])/(.+?)\( *(\d+)\): (.*?)$') | |||
| if args.add_timestamp: | |||
There was a problem hiding this comment.
It doesn't make sense to have this logic sprinkled all over. Always capture the timestamp and just have the condition on whether or not it gets displayed.
|
I don't like this output. While you fixed my issues with the timestamp itself simply prepending it to the message is ugly as demonstrated in the original PR. |
|
What needs to be done for this to be accepted? I really like pidcat, but the absence of a timestamp is very frustrating. |
|
@BoD I think Jake wants the timestamp to be formatted in a separate column and made visually distinct (like the tags and log levels). See the discussion on the original PR. I just haven't had time to get to it. |
|
@JakeWharton @vickychijwani @marczych @BoD Has something changed here?? |
|
@jude07 None of the timestamp pulls have been merged as far as I know so you'll have to use one of the forks to get that option. |
Enabled with the
--timestampflag. Fixes #9.All I've done is rebase @marczych's commit in PR #34 to the latest master. I've also removed the short
-tflag because it's already being used for tags. Let me know if any other changes are required to get this merged in.