Skip to content
This repository has been archived by the owner on Aug 3, 2024. It is now read-only.

Check in first pass of a "-f" option for ProblemsCommand which #293

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cmore-zz
Copy link

@cmore-zz cmore-zz commented Mar 9, 2014

Background

For my day job, I work on a large old codebase that has generates a bazillion (okay, about 40K) eclipse warnings at a reasonable warning level. That's an overwhelming number, but I like to correct warnings when I'm visiting a file for a bug fix or feature addition. So, as an emacs-eclim user I wanted to add an option to filter warnings down to the current file.

This changes treats a -f option as specifying a source file to limit warnings to. It has no effect on error reporting.

Issues

  • The meaning of the "-f" option seems a little odd, since it only affects warning (and one might reasonably expect it to affect both). I'd be fine specifying it differently, (maybe via a "-fw" option, or something), but the all errors plus warnings for this particular file is a useful combo for an older codebase like my employer's. I'm happy to use a different option (or combination of options) to achieve the same effect.
  • I'm not very familiar with the eclim and eclipse programming, so I don't know if there's a better way do a file compare besides using java.io.File to get the canonical path. I try to minimize the calls by only doing them if "-f" is specified, and if the file for the warning has changed from the previous warning. However, there might be a better/more-efficient way to do so in eclipse. In practice, it seems reasonably fast (and certainly much faster than emitting and parsing 40k warnings as JSON).
  • I see I accidentally change indent level on two lines of a commit. I will redo, if need be.

filters warnings (not errors) to those for the current file.
@ervandew
Copy link
Owner

I just got a chance to look at this and I have a couple of questions:

  1. So you want to have the ability to filter out warnings from the project problems command?
    That alone makes sense to me, and I would think that adding a setting to disable warnings would be the best approach here.
  2. But you want to show warnings for the current file? Why not just use the java_src_update command to view all the errors and warnings for the current file and only use project problems for viewing errors?

It feels weird to me to have a -f argument on a command that operates project wide. I'm not familiar with how emacs-eclim utilizes the java_src_update vs the problems command so I don't know what a typical workflow in emacs would look like.

@cmore-zz
Copy link
Author

First of all, thanks for your response and getting back to me. I appreciate you taking the time to do so.

So, emacs-eclim, like eclipse itself, only has a single place for problems to display (for emacs-eclim, this is a "*eclim: problems_" buffer). The JSON returned from java_src_update is not currently acted upon by emacs-eclim. Instead, a complete update of the _eclim:problems* buffer is queued pending idle.

This is similar to eclipse, where there's generally a single global "problems" tab. In the eclipse UI, one can change (via configuration) how global or local the contents of that tab are. For example, I have mine set such that warnings are local to the current file or selection, but errors are global. I'd like to have the equivalent behavior in emacs, so I don't have to go back to the eclipse UI (which I dislike) to see if there are warnings I can fix while I'm in the area.

So, given all that, I don't think changes to what java_src_update returns is going to be helpful for emacs-eclim without creating some additional new functionality (such as an file-specific problems buffer, or something). While I think that might be worthy functionality (and likely would give a quicker response than issuing a separate problems command), it's pretty different from how emacs-eclim works today.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants