Skip to content

Indicate when one or more lint job is running #712

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

Closed
wants to merge 4 commits into from
Closed

Indicate when one or more lint job is running #712

wants to merge 4 commits into from

Conversation

aomader
Copy link

@aomader aomader commented Jul 8, 2015

The bottom bar is updated to show if at least one lint job is running. This is very helpful with larger projects where lint jobs might take some time. This is just my idea how one could visualize this state.

Since this is my first contact with Linter, I would argue that you should very extensive check that everything is in order.

@@ -32,6 +33,9 @@ class Linter

@subscriptions.add atom.workspace.observeTextEditors (editor) =>
currentEditorLinter = new EditorLinter @, editor
currentEditorLinter.onLinting (linting) =>
@lintJobs += if linting then 1 else -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if linting then @lintsJobs++ else @lintsJobs--

But I think it's just preference of how to write it. (I find it clearer)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find yours cleaner as well. It's just too late 😴

@aomader
Copy link
Author

aomader commented Jul 8, 2015

Added the proposed simplification and removed the fields prefixed with an underscore.

@steelbrain
Copy link
Owner

while this PR looks good, you are not actually counting active linters, You are counting active editors that have linting in progress.

@aomader
Copy link
Author

aomader commented Jul 9, 2015

@steelbrain You're right. What's wrong about that?

@steelbrain
Copy link
Owner

@b52 the users will expect to see a number of active linters instead of active editor linters IMO

@steelbrain
Copy link
Owner

Showing linters instead of editor linters there should be a fairly simple change, all you have to do is increment or decrement in EditorLinter::triggerLinters's foreach loop.

@aomader
Copy link
Author

aomader commented Jul 11, 2015

@steelbrain: Pardon? I don't show any linters, thus the actual number doesn't matter. It's just important to check if any linter is running, and if so, show it. That is what I do, since I use the Promise::all as source applied on your EditorLinter::triggerLinters outcome, i.e., the array or linter promises.

So, what is left for me to do?

PS: I just made the sync icon rotate to illustrate even more that something is happening.

@steelbrain
Copy link
Owner

LGTM. /cc @AtomLinter/linter

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.

3 participants