[Android] LogcatMonitor.FindAll returns regex match objects

[Android] LogcatMonitor.FindAll returns regex match objects

Match objects are more convenient since they would also allow the caller
to define further groups to retrieve in the message_regex, or have
access to the full line being matched.

BUG=

Committed: https://crrev.com/b3d3e418b8b7d8c0d7dc13680aebb97283de06d9
Cr-Commit-Position: refs/heads/master@{#321327}

[perezju, 2015-03-18 16:21:20]

perezju@chromium.org changed reviewers:
+ jbudorick@chromium.org

[perezju, 2015-03-18 16:21:21]

I did a quick search for logcat_monitor FindAll, and I think I'm the only
potential user for this so far. So I propose a small interface change to make it
give a more useful return value.

[jbudorick, 2015-03-18 16:58:41]

https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
File build/android/pylib/device/logcat_monitor.py (right):

https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
build/android/pylib/device/logcat_monitor.py:101: Yields:
Why is this more useful? Why is it easier to do:

  for m in logmon.FindAll(...):
    m.group('log_level')

than

  for m in logmon.FindAll(...):
    m.log_level

[jbudorick, 2015-03-18 17:02:26]

On 2015/03/18 16:58:41, jbudorick wrote:
>
https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
> File build/android/pylib/device/logcat_monitor.py (right):
> 
>
https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
> build/android/pylib/device/logcat_monitor.py:101: Yields:
> Why is this more useful? Why is it easier to do:
> 
>   for m in logmon.FindAll(...):
>     m.group('log_level')
> 
> than
> 
>   for m in logmon.FindAll(...):
>     m.log_level

also, in what circumstance would the caller want the full line?

[perezju, 2015-03-18 17:38:41]

https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
File build/android/pylib/device/logcat_monitor.py (right):

https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
build/android/pylib/device/logcat_monitor.py:101: Yields:
On 2015/03/18 16:58:40, jbudorick wrote:
> Why is this more useful? Why is it easier to do:
> 
>   for m in logmon.FindAll(...):
>     m.group('log_level')
> 
> than
> 
>   for m in logmon.FindAll(...):
>     m.log_level

I want to be able to write, e.g.:

    for m in logmon.FindAll(r'>>> (?P<crash_process>.+) <<<', log_level='I',
component='DEBUG'):
      logging.warning('%s crashed', m.group('crash_process'))

[jbudorick, 2015-03-18 17:43:43]

https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
File build/android/pylib/device/logcat_monitor.py (right):

https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
build/android/pylib/device/logcat_monitor.py:101: Yields:
On 2015/03/18 17:38:41, perezju wrote:
> On 2015/03/18 16:58:40, jbudorick wrote:
> > Why is this more useful? Why is it easier to do:
> > 
> >   for m in logmon.FindAll(...):
> >     m.group('log_level')
> > 
> > than
> > 
> >   for m in logmon.FindAll(...):
> >     m.log_level
> 
> I want to be able to write, e.g.:
> 
>     for m in logmon.FindAll(r'>>> (?P<crash_process>.+) <<<', log_level='I',
> component='DEBUG'):
>       logging.warning('%s crashed', m.group('crash_process'))

I can see the case for wanting named capture groups in the message. Would that
work with the current implementation? You'd wind up attempting to compile:

  '(?P<message>>>> (?P<crash_process>.+) <<<)$'

(I've never tried nested named capture groups.)

[perezju, 2015-03-18 17:55:01]

On 2015/03/18 17:43:43, jbudorick wrote:
>
https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
> File build/android/pylib/device/logcat_monitor.py (right):
> 
>
https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
> build/android/pylib/device/logcat_monitor.py:101: Yields:
> On 2015/03/18 17:38:41, perezju wrote:
> > On 2015/03/18 16:58:40, jbudorick wrote:
> > > Why is this more useful? Why is it easier to do:
> > > 
> > >   for m in logmon.FindAll(...):
> > >     m.group('log_level')
> > > 
> > > than
> > > 
> > >   for m in logmon.FindAll(...):
> > >     m.log_level
> > 
> > I want to be able to write, e.g.:
> > 
> >     for m in logmon.FindAll(r'>>> (?P<crash_process>.+) <<<', log_level='I',
> > component='DEBUG'):
> >       logging.warning('%s crashed', m.group('crash_process'))
> 
> I can see the case for wanting named capture groups in the message. Would that
> work with the current implementation? You'd wind up attempting to compile:
> 
>   '(?P<message>>>> (?P<crash_process>.+) <<<)$'
> 
> (I've never tried nested named capture groups.)

Yes. Just tested. The line

  '03-18 16:45:58.377   189   189 I DEBUG   : pid: 2813, tid: 2813, name:
.android.chrome  >>> com.android.chrome <<<'

will produce the m.groupdict()

  {'log_level': 'I', 'proc_id': '189', 'component': 'DEBUG', 'thread_id': '189',
'time': '16:45:58.377', 'date': '03-18', 'message': 'pid: 2813, tid: 2813, name:
.android.chrome  >>> com.android.chrome <<<', 'crash_process':
'com.android.chrome'}

[jbudorick, 2015-03-18 17:56:52]

On 2015/03/18 17:55:01, perezju wrote:
> On 2015/03/18 17:43:43, jbudorick wrote:
> >
>
https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
> > File build/android/pylib/device/logcat_monitor.py (right):
> > 
> >
>
https://codereview.chromium.org/1007423003/diff/1/build/android/pylib/device/...
> > build/android/pylib/device/logcat_monitor.py:101: Yields:
> > On 2015/03/18 17:38:41, perezju wrote:
> > > On 2015/03/18 16:58:40, jbudorick wrote:
> > > > Why is this more useful? Why is it easier to do:
> > > > 
> > > >   for m in logmon.FindAll(...):
> > > >     m.group('log_level')
> > > > 
> > > > than
> > > > 
> > > >   for m in logmon.FindAll(...):
> > > >     m.log_level
> > > 
> > > I want to be able to write, e.g.:
> > > 
> > >     for m in logmon.FindAll(r'>>> (?P<crash_process>.+) <<<',
log_level='I',
> > > component='DEBUG'):
> > >       logging.warning('%s crashed', m.group('crash_process'))
> > 
> > I can see the case for wanting named capture groups in the message. Would
that
> > work with the current implementation? You'd wind up attempting to compile:
> > 
> >   '(?P<message>>>> (?P<crash_process>.+) <<<)$'
> > 
> > (I've never tried nested named capture groups.)
> 
> Yes. Just tested. The line
> 
>   '03-18 16:45:58.377   189   189 I DEBUG   : pid: 2813, tid: 2813, name:
> .android.chrome  >>> com.android.chrome <<<'
> 
> will produce the m.groupdict()
> 
>   {'log_level': 'I', 'proc_id': '189', 'component': 'DEBUG', 'thread_id':
'189',
> 'time': '16:45:58.377', 'date': '03-18', 'message': 'pid: 2813, tid: 2813,
name:
> .android.chrome  >>> com.android.chrome <<<', 'crash_process':
> 'com.android.chrome'}

Nice. That seems useful. lgtm.

[perezju, 2015-03-19 09:07:29]

The CQ bit was checked by perezju@chromium.org

[perezju, 2015-03-19 09:07:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/1007423003/#ps20001
(title: "add optional space after component")

[commit-bot: I haz the power, 2015-03-19 09:07:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1007423003/20001

[commit-bot: I haz the power, 2015-03-19 10:18:32]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-19 10:19:13]

Patchset 2 (id:??) landed as
https://crrev.com/b3d3e418b8b7d8c0d7dc13680aebb97283de06d9
Cr-Commit-Position: refs/heads/master@{#321327}