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

change to a one-weed-per-line output format #62

Merged
merged 8 commits into from Apr 30, 2021

Conversation

elaforge
Copy link
Contributor

This makes it easier to read. Also it lets me suppress duplicates with
uniq.

This makes it easier to read.  Also it lets me suppress duplicates with
uniq.
@ocharles
Copy link
Owner

I'm fairly allergic to flags and wonder if this could just be the new default output format. I think I'd be fine with that, and it would simplify the codebase.

@elaforge
Copy link
Contributor Author

I also like that idea. No information is lost with the concise format except the context, and I don't need that since if I'm going to pull the weed I need to go there in the editor anyway. I added a new commit, not sure if you prefer to squash or not.

@elaforge elaforge changed the title add a --concise flag with a one-weed-per-line output format change to a one-weed-per-line output format Mar 16, 2021
@ocharles
Copy link
Owner

I've finally had a chance to try this out and it works great! I do wonder if we're maybe being a little too terse though. How about

src/Weeder.hs:403: foo
src/Weeder.hs:401: unused

Becomes

src/Weeder.hs:403: Defined but not used: `foo`
src/Weeder.hs:401: Defined but not used: `unused`

Yes, it's a bit repetitive - but I feel also a bit more friendly. Any thoughts?

@elaforge
Copy link
Contributor Author

I'd still go for maximum terse. If weeder ever said anything other than "Defined but not used" that would be a different matter, but after the 50th time (or even just 3rd time), it just becomes the noise in between what you're looking for, which is where and what. At that point a more distinctive divider like |---------| would be less distracting!

Also you might have this in a vim quickfix window or something, where space is tight. Ok well that's tight on vertical space not horizontal so no big deal. Maybe someone wants to wrap their quickfix in columns :) Anyway that's a stretch so it's up to you, I think one per line is already just fine.

@ocharles
Copy link
Owner

Ok, let's go with this then

@ocharles ocharles enabled auto-merge (squash) April 30, 2021 08:27
@ocharles ocharles merged commit 7ee5fe9 into ocharles:master Apr 30, 2021
ocharles added a commit that referenced this pull request Apr 30, 2021
Since #62, we no longer need to store source code during analysis.
@ocharles ocharles mentioned this pull request Apr 30, 2021
ocharles added a commit that referenced this pull request Apr 30, 2021
Since #62, we no longer need to store source code during analysis.
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

2 participants