Skip to content
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

Filter By Tag #43

Closed
wants to merge 2 commits into from
Closed

Filter By Tag #43

wants to merge 2 commits into from

Conversation

adstro
Copy link
Contributor

@adstro adstro commented Jan 10, 2014

I added this code before I found the "--always-display-tags" switch which allows for useful grepping. I don't know if it's useful, but thought I would pass it along anyways.

@JakeWharton
Copy link
Owner

Doesn't the output look really odd since there's never anything on the left?

@adstro adstro closed this Jan 10, 2014
@adstro adstro reopened this Jan 10, 2014
@adstro
Copy link
Contributor Author

adstro commented Jan 10, 2014

It does, so I was going to add a switch to force the logging of the tag...thats when I found "--always-display-tags" ;). If you use the two switches together, the output does not look bad. But since, they both exist, I didn't force -t to imply "--always-display-tags" (thats a tiny change though)

@JakeWharton
Copy link
Owner

Is there a specific reason you aren't just using grep?

@adstro
Copy link
Contributor Author

adstro commented Jan 10, 2014

Only because I didn't know that always-display-tags existed until I made the changes. As I said in the request, I don't know if its useful, but thought I would pass it along since I already did it. Feel free to discard the changes.

@JakeWharton
Copy link
Owner

Let me think on it a bit.


Jake Wharton
http://about.me/jakewharton

On Thu, Jan 9, 2014 at 9:13 PM, Adam Stroud notifications@github.comwrote:

Only because I didn't know that always-display-tags existed until I made
the changes. As I said in the request, I don't know if its useful, but
thought I would pass it along since I already did it. Feel free to discard
the changes.


Reply to this email directly or view it on GitHubhttps://github.com//pull/43#issuecomment-32003238
.

@adstro
Copy link
Contributor Author

adstro commented Jan 10, 2014

No worries, I'm cool with whatever you want to do.

@romainpiel
Copy link

I'm jumping in this conversation. The problem with grep is that it won't print everything if the log text is on multiple lines. I would love to be see something like --tag=MY_TAG
Awesome work by the way, I use pidcat every days.

@tomrozb
Copy link

tomrozb commented Jul 17, 2014

@romainpiel you can try my fork of the pidcat https://github.com/tomrozb/pidcat You can specify tag or/and tag prefix if you like

@romainpiel
Copy link

@tomrozb that's great I'll try it out, thanks!

@@ -40,6 +40,7 @@
parser.add_argument('-s', '--serial', dest='device_serial', help='Device serial number (adb -s option)')
parser.add_argument('-d', '--device', dest='use_device', action='store_true', help='Use first device for log input (adb -d option).')
parser.add_argument('-e', '--emulator', dest='use_emulator', action='store_true', help='Use first emulator for log input (adb -e option).')
parser.add_argument('-t', '--tag', dest='tag', help='Filter output by specified tag')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have action='append' so it can be specified multiple times. Below should check with in (since it'll be a list) rather than equality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JakeWharton I just updated the code to support a list of tags. Should I submit a new pull request?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can push to this PR's branch and it will update. Looks like you also need to rebase on the latest changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change was made in a new git repo. I deleted the old repo when I was doing some house cleaning. Do you know it that will that cause an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JakeWharton I created a new PR (#68) that contains the changes from the new repo. If we still want the changes, we can either merge that one, or merge this one as-is and I can address the comments in another commit.

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.

None yet

4 participants