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

Enable use of Vim's quickfix window and jump to build error features #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

davisking
Copy link

This change allows the use of vim's built in quickfix window with Bazel build. For example, the vim command :Bazel build //whatever:thing still builds as before, however, you can run :cwindow to see the errors and jump to them after the build finishes. It also sets makeprg so you can then quickly rerun the same build by simply invoking :make. This is especially convenient since existing macros that use :make to initiate a build now work sensibly in this context.

I've also seen the discussion on #1 about how vim's error parsing isn't up to the task here. However, I haven't found there to be any issues. I did need to modify the default errorformat parsing rules in vim a bit, but with some minor tweaks they are parsing compiler errors just fine. I did only test it with gcc on linux, so maybe the issues being discussed in #1 are with regards to some other setup.

This change allows the use of vim's built in quickfix window with Bazel
build. For example, the vim command :Bazel build //whatever:thing still
builds as before, however, you can run :cwindow to see the errors and
jump to them after the build finishes. Since it also sets makeprg you
can then rerun the same build by simply invoking :make.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@davisking
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@alexkohler
Copy link

Thanks a bunch for this - I spent a good bit of time wrestling with my quickfix window before finding this PR 😄Any reason this hasn't been looked at?

@dbarnett
Copy link
Contributor

Yup, see discussion in #1 and the comment in the description here:

I've also seen the discussion on #1 about how vim's error parsing isn't up to the task here.

Bazel is a frontend for a lot of tools and we've gotten stuck trying to support some of those using errorformat (in particular multiline errors from javac, IIRC). Hop on the discussion in #1 and maybe you can help us find a simple path forward here that we've missed.

@mellery451
Copy link

thanks @davisking for creating this PR. I'm going to steal the errorformats you created and just define my own vim compiler file for bazel...that seems like the most workable solution to getting quickfix as well as async support.

@jake-arkinstall
Copy link

Thanks for the PR. I'm also going to steal the errorformats.

In terms of the project, though, I can't see the justification for holding back the PR. The entire point of open source is that a new feature is rarely perfect. The most common approach I have seen for awkward problems like this is for half-solutions to be pulled so that people can try it out with their own environment, identify problems, add issues, add pull requests, and improve it. They can't do that if there's nothing to improve on. Baby steps are better than no steps, and given that this project has been stale for almost a year, I think it's about time they were made.

I also completely disagree with the idea that quickfix isn't the right paradigm. Again, it's not perfect. But something that people are already used to, even if the implementation is awkward, is infinitely better than having nothing.

dbarnett added a commit that referenced this pull request Aug 22, 2020
@dbarnett
Copy link
Contributor

Welcome! I do encourage you to tinker with the plugin locally and "steal" whatever solutions work for you, and I've just added a note pointing to this PR for anyone who might be served just fine by the changes as-is.

As far as what gets merged into the repo, I really need any functionality to be maintainable in practice, and in this case I'm not convinced that there's a viable path to evolve it from adding some quick errorformat regexes to solving all the follow-on complaints and feature requests users would report against that. I could be mistaken. If anyone wants to hop into #1 and share some "existence proofs" on how it could be extended to handle output from more interesting tools without becoming a maintainability nightmare, please go for it.

@jake-arkinstall
Copy link

As far as what gets merged into the repo, I really need any functionality to be maintainable in practice, and in this case I'm not convinced that there's a viable path to evolve it from adding some quick errorformat regexes to solving all the follow-on complaints and feature requests users would report against that. I could be mistaken. If anyone wants to hop into #1 and share some "existence proofs" on how it could be extended to handle output from more interesting tools without becoming a maintainability nightmare, please go for it.

Okay, I'll look into that. The errorformat regexes (as horrible as they are) are already available for many languages in various FOSS projects, so honestly I think it'd just mean curating a list, and selecting the appropriate regexes for the build. Extension of that could be done quite effectively by the community via pull request.

An alternative approach, and it's probably the easiest to work with, is to modify the make command to run through a parser - something that runs Bazel, but then collects the output, re-processes it to some standard format, and then just have an errorformat regex for that standard format.

One step further (but unlikely to happen because of the coordination required) is to have that in Bazel itself. Have the maintainers for each different language add a parser for the error output for the various compilers that they support. Nothing particularly fancy, no interpretation of errors, just a reformatting of them. This way, the error output of a bazel build would be much easier to work with not just with vim, but even by eye, or with any IDE.

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.

6 participants