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

I'd like to be able to use next-error and previous-error in Emacs with diff-cover's output #51

Merged
merged 3 commits into from Jun 25, 2018

Conversation

glyph
Copy link
Contributor

@glyph glyph commented Sep 25, 2016

This is similar to -f parseable from pylint - which, oddly, diff-cover appears to require although it can't output :)

@Bachmann1234
Copy link
Owner

Looks good, tests broke due to the change in output but should be easy to fix.

Thanks for the PR!

@glyph
Copy link
Contributor Author

glyph commented Sep 25, 2016

@Bachmann1234 thanks! I was mainly filing this as a feature-request; any pointers on where I should look to fix the tests?

@Bachmann1234
Copy link
Owner

Sure! So the diff-cover/diff-quality test suite is... integration test heavy. A lot of the tests depend on the output of specific commands/reports.

So your pr modified the console report template.

Looking at the failures updating these files would fix things

pep8_violations_report.txt
pyflakes_violations_report.txt
pyflakes_two_files.txt
pylint_violations_console_report.txt
add_console_report.txt
pylint_dupe_violations_report.txt

It may not be obvious what the src_path should be in the errors. But I guess you can just see what is outputting and base your changes on that. Make sense? Or you can look at the corresponding diff fixture the test loads up.

@Bachmann1234
Copy link
Owner

As a separate question. Is there any other work I would need to support this emacs feature? Or does this PR cover it. I am fairly ignorant of Emacs but if you know the things that can be done I can make a note to do them when I get around to it.

Though full disclosure, its looking like a super busy few months for me so for the most part im mostly in "triage" mode.

@glyph
Copy link
Contributor Author

glyph commented Sep 26, 2016

As a separate question. Is there any other work I would need to support this emacs feature? Or does this PR cover it. I am fairly ignorant of Emacs but if you know the things that can be done I can make a note to do them when I get around to it.

Nope, emacs has a pretty standard regex that will work if you run stuff from M-x compile.

@glyph
Copy link
Contributor Author

glyph commented Dec 30, 2017

Was this just closed because it has been stale for a while or is there a different fix available?

@Bachmann1234
Copy link
Owner

Mostly cuse its stale.... looking back over the changes there actually may not be a need to close. I was closing old PRs because the code had changed a fair amount... But this change is fairly simple... Let me reopen. I cant promise anything but perhaps I can find sometime in January to give this some love.

@Bachmann1234 Bachmann1234 reopened this Dec 31, 2017
@glyph
Copy link
Contributor Author

glyph commented Jan 7, 2018

Thanks a ton!

@glyph
Copy link
Contributor Author

glyph commented Jun 24, 2018

This is now stacked on top of #76 so that should probably be merged first, but I believe the tests should pass now.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 99.057% when pulling 17337bc on glyph:master into 85c3095 on Bachmann1234:master.

@coveralls
Copy link

coveralls commented Jun 24, 2018

Coverage Status

Coverage remained the same at 99.097% when pulling 17337bc on glyph:master into 85c3095 on Bachmann1234:master.

@Bachmann1234
Copy link
Owner

Nice! Ill look this over either later tonight or tomorrow!

@Bachmann1234 Bachmann1234 merged commit cc89b18 into Bachmann1234:master Jun 25, 2018
@Bachmann1234
Copy link
Owner

https://pypi.org/project/diff_cover/1.0.3/ released!

@glyph
Copy link
Contributor Author

glyph commented Jul 18, 2018

Thanks so much for bearing with me over such a long period of time :)

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

3 participants