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

Ignore invalid TextEditors for fix jobs #978

Merged
merged 1 commit into from
Aug 9, 2017

Conversation

Arcanemagus
Copy link
Member

When a fix job is triggered but the TextEditor is invalid by the time the job runs silently return.

Fixes #972.

@Arcanemagus Arcanemagus added the bug label Aug 9, 2017
@Arcanemagus Arcanemagus requested a review from IanVS August 9, 2017 06:19
@Arcanemagus
Copy link
Member Author

Blocked on #979.

if (!textEditor || textEditor.isModified()) {
if (!textEditor || !atom.workspace.isTextEditor(textEditor)) {
// Silently return if the TextEditor is invalid
return
Copy link
Member

Choose a reason for hiding this comment

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

We talked about maybe logging something out to the console, did you decide not to?

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue I see with that is the initially reported scenario: Making changes to dozens (if not hundreds) of files at once. If that's what was happening your console would suddenly be spammed with messages with no way to determine what went wrong, or how to fix it (which you couldn't anyway here).

Copy link
Member

@IanVS IanVS left a comment

Choose a reason for hiding this comment

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

LGTM, just needs a rebase on master it seems.

When a fix job is triggered but the TextEditor is invalid by the time
the job runs silently return.

Fixes #972.
@Arcanemagus Arcanemagus force-pushed the arcanemagus/ignore-invalid-fix-editor branch from aa609e6 to 5d823d6 Compare August 9, 2017 16:40
@Arcanemagus Arcanemagus merged commit 5a8465a into master Aug 9, 2017
@Arcanemagus Arcanemagus deleted the arcanemagus/ignore-invalid-fix-editor branch August 9, 2017 16:55
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