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

add go to previous error #678

Conversation

despairblue
Copy link
Contributor

No description provided.

@despairblue despairblue force-pushed the feature/previous-error branch from f030c7f to 1e3862d Compare July 1, 2015 01:19
@despairblue despairblue force-pushed the feature/previous-error branch from 1e3862d to 8a5947b Compare July 1, 2015 09:16
message = next.value
return unless message.filePath
return unless message.range
if @index?
Copy link

Choose a reason for hiding this comment

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

Seems like we should really be indexing errors by some sort of hash of position and file name. Otherwise changes to the full set of messages would not have a stable order. This is of course assuming our linter messages have a stable order.

On that note we should really have a stable order, because it doesn't look like multiple linters actually have a stable order in our error processing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I tried it with one linter, that was stable, but if there are tow and they report asynchronously it's going to be a problem as soon as one changes something.

The behavior before this PR was to always jump to the first reported error after an edit.

@park9140
Copy link

park9140 commented Jul 1, 2015

🚢 Seems fine other than the overarching issue of usability. We should really add a story to address the order of errors across linter.

@despairblue
Copy link
Contributor Author

I extracted the modulo into a method and added an explanation.

@steelbrain
Copy link
Owner

Merging based on review from @park9140

steelbrain added a commit that referenced this pull request Jul 3, 2015
@steelbrain steelbrain merged commit 9489431 into steelbrain:steelbrain/compact-storage Jul 3, 2015
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