Closed
Bug 558783
Opened 14 years ago
Closed 13 years ago
Add "smart move" behaviour back
Categories
(Skywriter Graveyard :: Editor, defect)
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: julian.viereck, Unassigned)
Details
Attachments
(2 files, 1 obsolete file)
In TM, you can activate the "Soft Tab" setting. If turned on, tabstop spaces are treated as tabs. This feature hasn't been added back to the rebooted Bespin yet.
|
||
Comment 1•14 years ago
|
||
TM'S SmartMove implementation which has the cursor jump over *tab* spaces when left and right are pressed (moving the cursor around) and the cursor hits a spot where tab was pressed is encountered.
Attachment #438575 -
Flags: review?(jviereck)
|
Reporter | |
Comment 2•14 years ago
|
||
Hi Derek, a few things: a) The behavior is not 100% correct implemented. If the cursor is in a line that looks like this 4 x <space> Hello and the cursor is at column 2, then pressing ARROW_RIGHT should move the cursor 2 steps to the right if the tabstop is 4. Your implementation always moves n-tabstops in one direction. b) The behavior is only implemented for the moveLeft/moveRight function. It would be great to have this for every kind of left/right movement (selection etc...). The left/right move functions call textStorage.displacePosition(). Maybe you could add a function "_replacePosition(pos, count, smartMove)" to the textView that handles smartMovement if the passed argument smartMove is turned on? c) Some people don't want this feature to be turned on. Could you add a setting for this like "smartMove"? Hint: You have to define settings before you can use them - take a look at the text_view/package.json file and search for "setting". Beside this, please make sure you setup your HG correctly - the patch you uploaded was not "git" formated (https://developer.mozilla.org/en/Mercurial_FAQ). Cheers Julian
|
|
||
Comment 3•14 years ago
|
||
Update to the previous patch. Is this heading in the right direction?
Attachment #441339 -
Flags: review?(jviereck)
|
|
Reporter | |
Comment 4•14 years ago
|
||
I can change the setting, that part looks good. However, the setting doesn't seem to change anything - smart movement is still enabled when smartmove is false. While testing the code I've noticed the following things: a) The things noticed in comment #2 seem still to apply b) When pressing ARROW_RIGHT within a word, the cursor moves to the left c) If the line above the current line and the following line looks like this <body> |<space><space><space><space>Hello And I press the ARROW_LEFT key, the cursor goes to <bo|dy> <space><space><space><space>Hello I expect the cursor to be at <body>| d) I just took a short look at your code. I don't really understand the moveLeft and moveRight code. What's that rcount variable for? Julian
|
|
||
Comment 5•14 years ago
|
||
I'm really sorry about that Julian. I must have uploaded a previous/scrapped version. This is the one I meant for you to review.
Attachment #441339 -
Attachment is obsolete: true
Attachment #441441 -
Flags: review?(jviereck)
Attachment #441339 -
Flags: review?(jviereck)
|
|
Reporter | |
Comment 6•14 years ago
|
||
Hi Derek,
no worries about that. Thanks for your work so far.
The setting works.
I've noticed the following behaviors that should get fixed:
1) Let's say the current line looks like this and tabstop = 4:
<space><space><space><space>|<space><space>Hello world
Where | is the cursor position. When the user hits the right arrow key, the cursor goes here:
<space><space><space><space><space><space>He>>|<<llo world
(I've added the >> and << to highlight the cursor a little bit more). The cursor should just move one more position to the right, like this:
<space><space><space><space><space>|<space>Hello world
2) Smart movement only works at the beginning of the line, but not for the end or some text in between like this:
Hello world<space><space><space><space><space><space><space>|<space><space>
3) While taking a short look at your code I've noticed that you're using RegExp to test for the number of free characters. I recommend to execute a RegExp to get the number of spaces. That way you don't have to run two RegExp test and you don't have to create new RegExp objects each time (which is not as performant as using the same RegExp again).
4) You use two separate code paths. One for smartmove on and one for smartmove off. I think the code would look much cleaner if you could unit these two things. Actually, if you turn smartmove on and _rangeIsInsertionPoint(range) is true, you just have to replace the cursor n times instead of 1. That could look like this:
var range = Range.normalizeRange(this._selectedRange);
if (this._rangeIsInsertionPoint(range)) {
var times;
if(smartmove == false) {
times = 1;
} else {
// calculate times...
}
this.moveCursorTo(this.getPath('layoutManager.textStorage').
displacePosition(range.end, times));
} else {
this.moveCursorTo(range.end);
}
Cheers
Julian
|
||
Comment 7•13 years ago
|
||
ACETRANSITION The Skywriter project has merged with Ajax.org's Ace project (the full server part of which is their Cloud9 IDE project). Background on the change is here: http://mozillalabs.com/skywriter/2011/01/18/mozilla-skywriter-has-been-merged-into-ace/ The bugs in the Skywriter product are not necessarily relevant for Ace and quite a bit of code has changed. For that reason, I'm closing all of these bugs. Problems that you have with Ace should be filed in the Ace issue tracker at GitHub: https://github.com/ajaxorg/ace/issues
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INVALID
|
|
Reporter | |
Updated•12 years ago
|
Attachment #438575 -
Flags: review?(julian.viereck)
|
|
Reporter | |
Updated•12 years ago
|
Attachment #441441 -
Flags: review?(julian.viereck)
|
|
Reporter | |
Comment 8•12 years ago
|
||
Remove review? request as the bug is RESOLVED INVALID.
You need to log in
before you can comment on or make changes to this bug.
Description
•