Add showCursorOnSelection config #13664

Merged
merged 1 commit into from Jan 20, 2017

Conversation

Projects
None yet
7 participants
@zertosh
Member

zertosh commented Jan 20, 2017

Description of the Change

Adds a new config editor.showCursorOnSelection, enabled by default (changing the current default behavior), that makes it so that the cursor is always blinking even when you have a selection(s).

Why Should This Be In Core?

It's pretty tied to the internals of Cursor and TextEditor.

Benefits

Brings Atom in line with the behavior of Sublime (and others).

This is super useful for when you do cmd+d (find-and-replace:select-next) and you can't find where your cursor went.

Possible Drawbacks

The current behavior is to hide the cursor when there's a selection. Since we're changing the default, that's sure to upset someone. However, since it is configurable, those that don't like it can easily revert to the old behavior.

@zertosh

This comment has been minimized.

Show comment
Hide comment
@zertosh

zertosh Jan 20, 2017

Member

To see what this behavior is like, you can quickly monkeypatch Cursor.prototype.updateVisibility with:

require(process.resourcesPath + '/app.asar/src/cursor.js')
  .prototype.updateVisibility = function() {
    this.setVisible(true);
  };
Member

zertosh commented Jan 20, 2017

To see what this behavior is like, you can quickly monkeypatch Cursor.prototype.updateVisibility with:

require(process.resourcesPath + '/app.asar/src/cursor.js')
  .prototype.updateVisibility = function() {
    this.setVisible(true);
  };
@zertosh

This comment has been minimized.

Show comment
Hide comment
@zertosh

zertosh Jan 20, 2017

Member

Test failures are unrelated. See d50b56b#commitcomment-20549225

Member

zertosh commented Jan 20, 2017

Test failures are unrelated. See d50b56b#commitcomment-20549225

@BinaryMuse

This comment has been minimized.

Show comment
Hide comment
Member

BinaryMuse commented Jan 20, 2017

@lee-dohm

This comment has been minimized.

Show comment
Hide comment
@lee-dohm

lee-dohm Jan 20, 2017

Member

Has any testing been done for when people change the appearance of their cursor? Examples:

Prevent cursor from blinking

atom-text-editor {
  .cursors.blink-off .cursor {
    opacity: 1;
  }
}

Make cursor pulse instead of blink

atom-text-editor {
  .cursor {
    transition: opacity 0.5s;
  }
}
Member

lee-dohm commented Jan 20, 2017

Has any testing been done for when people change the appearance of their cursor? Examples:

Prevent cursor from blinking

atom-text-editor {
  .cursors.blink-off .cursor {
    opacity: 1;
  }
}

Make cursor pulse instead of blink

atom-text-editor {
  .cursor {
    transition: opacity 0.5s;
  }
}
@zertosh

This comment has been minimized.

Show comment
Hide comment
@zertosh

zertosh Jan 20, 2017

Member

@lee-dohm, just tried both and they work just fine.

The change for the new behavior is just 4 lines (see https://github.com/atom/atom/pull/13664/files#diff-24f2fe2bd60cb4194c3cf7d107fa0f99L578). The bulk of this PR is tests and making it a setting that propagates correctly.

Member

zertosh commented Jan 20, 2017

@lee-dohm, just tried both and they work just fine.

The change for the new behavior is just 4 lines (see https://github.com/atom/atom/pull/13664/files#diff-24f2fe2bd60cb4194c3cf7d107fa0f99L578). The bulk of this PR is tests and making it a setting that propagates correctly.

@lee-dohm

This comment has been minimized.

Show comment
Hide comment
@lee-dohm

lee-dohm Jan 20, 2017

Member

Awesome, thanks @zertosh 🙇

Member

lee-dohm commented Jan 20, 2017

Awesome, thanks @zertosh 🙇

@nathansobo nathansobo merged commit 82741c7 into master Jan 20, 2017

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@nathansobo nathansobo deleted the fb-as-show-cursor-on-selection branch Jan 20, 2017

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Jan 20, 2017

Contributor

Looks super well-tested and clean.

Contributor

nathansobo commented Jan 20, 2017

Looks super well-tested and clean.

@iolsen

This comment has been minimized.

Show comment
Hide comment
@iolsen

iolsen Feb 3, 2017

Contributor

blinky-cursor

Contributor

iolsen commented Feb 3, 2017

blinky-cursor

@nicolassrod

This comment has been minimized.

Show comment
Hide comment
@nicolassrod

nicolassrod Apr 1, 2017

What is the name of the three vertical lines seen in the catch ?;
How can I have that on atom?

nicolassrod commented Apr 1, 2017

What is the name of the three vertical lines seen in the catch ?;
How can I have that on atom?

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Apr 2, 2017

Member

Indent Guide: Settings -> Editor -> Show Indent Guide

Member

50Wliu commented Apr 2, 2017

Indent Guide: Settings -> Editor -> Show Indent Guide

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