Lintingrefactoring #1808

Closed
wants to merge 21 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

mikechambers commented Oct 9, 2012

This is an initial pull request that includes a major refactoring of the linting code. It:

  • Moves the linting code into an extension
  • Removes references to linting code from core brackets code base
  • Includes Handlebars.js to handle templating (I think this should eventually replace mustache)

In addition, it contains a simple class for providing a standard interface for interacting with and creating bottom panels. Right now, the implementation is pretty basic, but I wanted to get feedback on the general approach before I did any additional work on it.

The Linting extension is built with the idea of supporting multiple lintiers, although it will require some additional work once we have another linter.

mikechambers added some commits Oct 7, 2012

Adding BottomPanel, which is a manager / wrapper for working with the…
… bottom panel in the editor. Preloading BottomPanel so it is available as a module.
check the document extension before linting to make sure it is suppor…
…ted type. Supported types include : html, css, json and JavaScript.
changed default for ignoring lines with whitespace to false, so now e…
…rrors will be show for any lines with trailing whitespace. Removed all trailing whitespace from checked in files.
Member

njx commented Oct 9, 2012

The issue with extracting all the linting code into an extension is that we don't have a way for extensions to refer to each other yet, so if we made this code pluggable, other extensions couldn't actually register with it. For now, I think it would make sense to leave the common linting code in the core app, and just factor out the JSLint-specific code into an extension.

Contributor

mikechambers commented Oct 10, 2012

Thinking about this some, it seems that the primary thing generic linting code would do is format the output for a bottom panel.

So, in that case we would have:

  • module that handles creating, presenting, controlling bottom panels
  • module that handle formatting and displaying (in a bottom panel) errors returning from linting / inspecting a file
  • extensions that provide specific linting implementations, such as jslint, and jshint

Is that what you were thinking?

Member

njx commented Oct 10, 2012

Exactly. In addition to the bottom panel, we could add management of gutter markers and (eventually) inline squiggles.

Sent from tiny phone

On Oct 9, 2012, at 10:08 PM, "Mike Chambers" <notifications@github.commailto:notifications@github.com> wrote:

Thinking about this some, it seems that the primary thing generic linting code would do is format the output for a bottom panel.

So, in that case we would have:

-module that handles creating, presenting, controlling bottom panels
-module that handle formatting and displaying (in a bottom panel) errors returning from linting / inspecting a file
-extensions that provide specific linting implementations, such as jslint, and jshint

Is that what you were thinking?


Reply to this email directly or view it on GitHubhttps://github.com/adobe/brackets/pull/1808#issuecomment-9290211.

@ghost ghost assigned jasonsanjose Oct 10, 2012

Member

jasonsanjose commented Oct 22, 2012

@mikechambers, just checking in with you. It looks like @njx and you proposed keeping linting code in core a module(s). How's it going? Do you want to keep this pull request open or should we close it out and wait for a new one?

Member

njx commented Oct 24, 2012

Looking at this again, I think this refactoring could be a good first step. To deal with the extension dependency issue, we could just take this existing code and just move it into a core folder, loading it on startup, instead of having it be in the extensions folder (similar to how we've integrated LiveDevelopment)--I think that wouldn't require any actual code changes to what's here already. If you do that, we could probably merge it as-is (pending code review).

The main blocker to merging is deciding on the templating issue. I'll continue the Google Group thread on that and see if we can resolve it soon.

The next step after merging this initial refactoring would be to replace the hardcoded reference to JSLINT with something that allowed you to register linting providers. At that point we could move the JSLint code into an extension.

Member

njx commented Oct 24, 2012

Actually, @redmunds pointed out that you can do the kind of loop you're doing in your template in Mustache. The syntax is similar--you just do {{#listname}} ... {{/listname}} instead of {{#each listname}} ... So maybe we could just rip Handlebars out from this pull request for now, and defer worrying about switching template libraries till later.

Member

njx commented Dec 6, 2012

Hi Mike--we had a discussion about this, and think it might be better to redo this starting from master by extracting just the JSLint portion into a linting provider extension, with the existing UI stuff remaining in the core. (Also, it looks like we now have a general resizable panel utility class, Resizer, for doing the panel management.) So we're closing this for now. Let us know if you'd like to work on it further. Thanks!

@njx njx closed this Dec 6, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment