Skip to content
This repository has been archived by the owner on Jan 5, 2021. It is now read-only.

Initial conversion to new decorations API #56

Closed
wants to merge 1 commit into from

Conversation

framerate
Copy link

Not sure if we need to save/destroy the markers too... and I feel like there should be a better way to clear them all than a loop and readding them. I looked through the API though and didn't see a "clearAllDecorations()" though. Anyone know one?

@Joezo
Copy link
Owner

Joezo commented Sep 8, 2014

I had a look at https://github.com/atom/decoration-example and it seems you're doing mostly the same things. The only thing i noticed that was different was that they saved/cached decorations per editor id, not just globally like in your implementation. Perhaps something to look at?

Also i'd like to keep the styling of the just changing the line number to red as it's not too intrusive? I've changed it back and am testing it locally today. I'll let you know how it goes!

@framerate
Copy link
Author

Yeah I'd rather have just line number too, but my first attempt at numbers didn't work and lines did so I just went with it for now :)

As to the editor ID, can't we just do something like :

var decorations = {};
if (typeof decorations[editorId] !== 'array') {
    decorations[editorId] = [];
}
decorations[editorId].push(decoration);

Then we could loop through all the key/values in the object to remove them.

@Joezo
Copy link
Owner

Joezo commented Sep 9, 2014

Yeah that sounds ideal. I changed the less file back and replaced on line 183 type: 'line' with type: 'gutter' to just do the line number :)

@Joezo
Copy link
Owner

Joezo commented Sep 9, 2014

So i've been using it today and had to disable it, it's causing a huge amount of lag. I'm guessing there is some sort of memory/performance issue, will need to look at it again.

@framerate
Copy link
Author

I know I tried in the beginning to make everyone work together on this project, but maybe it's time to officially support https://atom.io/packages/linter and https://atom.io/packages/linter-jshint

The project said it was designed to "stop the linter wars", so maybe we should just contribute to the jshint plugin there? Been using it for a few days and it's pretty good.

@dead-claudia
Copy link
Contributor

@framerate This is actually more stable than linter-jshint in my experience. My one source of crashes with this plugin disappeared with a simple update of JSHint. I still haven't pinpointed the cause of crashing in that plugin, since troubleshooting isn't particularly straightforward for it, and the errors are just getting in my way. I also like how this one doesn't clog up the screen upon error, which JSHint will inevitably spew errors to some extent if you're typing new code.

@Joezo
Copy link
Owner

Joezo commented May 22, 2015

I've pulled this into the branch fix-deprecations, which fixes deprecations :) Thanks for the help with this.

@Joezo Joezo closed this May 22, 2015
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.

3 participants