Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/base-config/keyboard.json
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,28 @@
"view.restoreFontSize": [
"Ctrl-0"
],
"view.scrollLineUp": [
{
"key": "Ctrl-Up",
"displayKey": "Ctrl-\u2191"
},
{
"key": "Ctrl-Alt-Up",
"displayKey": "Ctrl-Alt-\u2191",
"platform": "mac"
}
],
"view.scrollLineDown": [
{
"key": "Ctrl-Down",
"displayKey": "Ctrl-\u2193"
},
{
"key": "Ctrl-Alt-Down",
"displayKey": "Ctrl-Alt-\u2193",
"platform": "mac"
}
],
"navigate.quickOpen": [
"Ctrl-Shift-O"
],
Expand Down
2 changes: 2 additions & 0 deletions src/command/Commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ define(function (require, exports, module) {
exports.VIEW_INCREASE_FONT_SIZE = "view.increaseFontSize";
exports.VIEW_DECREASE_FONT_SIZE = "view.decreaseFontSize";
exports.VIEW_RESTORE_FONT_SIZE = "view.restoreFontSize";
exports.VIEW_SCROLL_LINE_UP = "view.scrollLineUp";
exports.VIEW_SCROLL_LINE_DOWN = "view.scrollLineDown";
exports.TOGGLE_JSLINT = "debug.jslint";
exports.SORT_WORKINGSET_BY_ADDED = "view.sortWorkingSetByAdded";
exports.SORT_WORKINGSET_BY_NAME = "view.sortWorkingSetByName";
Expand Down
10 changes: 9 additions & 1 deletion src/editor/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,15 @@ define(function (require, exports, module) {
Editor.prototype.setScrollPos = function (x, y) {
this._codeMirror.scrollTo(x, y);
};


/*
* Returns the current text height of the editor.
* @returns {number} Height of the text in pixels
*/
Editor.prototype.getTextHeight = function () {
return this._codeMirror.defaultTextHeight();
};

/**
* Adds an inline widget below the given line. If any inline widget was already open for that
* line, it is closed without warning.
Expand Down
2 changes: 2 additions & 0 deletions src/nls/root/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ define({
"CMD_INCREASE_FONT_SIZE" : "Increase Font Size",
"CMD_DECREASE_FONT_SIZE" : "Decrease Font Size",
"CMD_RESTORE_FONT_SIZE" : "Restore Font Size",
"CMD_SCROLL_LINE_UP" : "Scroll Line Up",
"CMD_SCROLL_LINE_DOWN" : "Scroll Line Down",
"CMD_SORT_WORKINGSET_BY_ADDED" : "Sort by Added",
"CMD_SORT_WORKINGSET_BY_NAME" : "Sort by Name",
"CMD_SORT_WORKINGSET_BY_TYPE" : "Sort by Type",
Expand Down
103 changes: 98 additions & 5 deletions src/view/ViewCommandHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@
define(function (require, exports, module) {
"use strict";

var Commands = require("command/Commands"),
CommandManager = require("command/CommandManager"),
Strings = require("strings"),
ProjectManager = require("project/ProjectManager"),
EditorManager = require("editor/EditorManager");
var Commands = require("command/Commands"),
CommandManager = require("command/CommandManager"),
KeyBindingManager = require("command/KeyBindingManager"),
Strings = require("strings"),
ProjectManager = require("project/ProjectManager"),
EditorManager = require("editor/EditorManager");

/**
* @const
Expand Down Expand Up @@ -126,7 +127,99 @@ define(function (require, exports, module) {
_removeDynamicFontSize(true);
}


/**
* @private
* Calculates the first and last visible lines of the focused editor
* @param {!Editor} editor
* @param {!number} scrollTop
* @param {!number} editorHeight
* @return {{first: number, last: number}}
*/
function _getLinesInView(editor, scrollTop, editorHeight) {
var textHeight = editor.getTextHeight(),
scrolledTop = scrollTop / textHeight,
scrolledBottom = (scrollTop + editorHeight) / textHeight;

// Subtract a line from both for zero-based index. Also adjust last line
// to round inward to show a whole lines.
var firstLine = Math.ceil(scrolledTop) - 1,
lastLine = Math.floor(scrolledBottom) - 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document what these magic numbers are.


return { first: firstLine, last: lastLine };
}

/**
* @private
* Scroll the viewport one line up or down.
* @param {number} -1 to scroll one line up; 1 to scroll one line down.
*/
function _scrollLine(direction) {
var editor = EditorManager.getCurrentFullEditor(),
scrollInfo = editor._codeMirror.getScrollInfo(),
textHeight = editor.getTextHeight(),
cursorPos = editor.getCursorPos(),
hasSelecction = editor.hasSelection(),
paddingTop = editor._getLineSpaceElement().offsetTop,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that Editor should have a new public method getPaddingTop that would return editor._getLineSpaceElement().offsetTop, and replaced _getLineSpaceElement. Checking through the code, the only other place where _getLineSpaceElement() is used is inside Editor to check the offsetTop, just like I did. Currently _getLineSpaceElement() is not public because is really specific to code mirror, but replacing it for getPaddingTop would make it less specific and could be changed to any other value if needed, even 0 if the padding would be removed at some point.

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding getPaddingTop sounds like a good idea. What about the bottom and sides? Maybe a getPadding method that returns an object with top, right, bottom, and left settings.

scrollTop = scrollInfo.top < paddingTop && direction > 0 ? paddingTop : scrollInfo.top,
scrolledTop = scrollTop,
editorHeight = scrollInfo.clientHeight,
linesInView = _getLinesInView(editor, scrollTop, editorHeight);

// Go through all the editors and reduce the scroll top and editor height to recalculate the lines in view
var line, total;
editor.getInlineWidgets().forEach(function (inlineEditor) {
line = editor._getInlineWidgetLineNumber(inlineEditor);
total = inlineEditor.info.height / textHeight;

if (line < linesInView.first) {
scrollTop -= inlineEditor.info.height;
linesInView = _getLinesInView(editor, scrollTop, editorHeight);

} else if (line + total < linesInView.last) {
editorHeight -= inlineEditor.info.height;
linesInView = _getLinesInView(editor, scrollTop, editorHeight);
}
});

// If there is no selection move the cursor so that is always visible
if (!hasSelecction) {
// Move the cursor to the first visible line
if (direction > 0 && cursorPos.line < linesInView.first) {
editor.setCursorPos({line: linesInView.first + 1, ch: cursorPos.ch});

// Move the cursor to the last visible line
} else if (direction < 0 && cursorPos.line > linesInView.last) {
editor.setCursorPos({line: linesInView.last - 1, ch: cursorPos.ch});

// Move the cursor up or down using CodeMirror function
} else if ((direction > 0 && cursorPos.line === linesInView.first) ||
(direction < 0 && cursorPos.line === linesInView.last)) {
editor._codeMirror.moveV(direction, "line");
Copy link
Contributor

Choose a reason for hiding this comment

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

This block of code seems to do (for the ===) the same as the 2 blocks above (for the > and < cases). Can't you change the first 2 blocks to be >= and <= and get rid of this block?

Or maybe use this block instead of the 2 blocks above, because I think the moveV() function handles variable-pitch fonts which Brackets might want some day.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could, but this block handles the goal column. Set cursor sets it to null internally, so it doesn't move from column 30 to 20 and then back to column 30, instead it would stay at column 20. Unfortunately the goal column is only kept when moving with the cursor inside the visible range, and lost on the first movement, but restored to a new one if not. I added this after a suggestion from @njx at #2343 (comment).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. Please add a comment to explain this.

}
}

// Scroll the editor
editor.setScrollPos(scrollInfo.left, scrolledTop + (textHeight * direction));
}


function _handleScrollLineUp() {
_scrollLine(-1);
}

function _handleScrollLineDown() {
_scrollLine(1);
}


CommandManager.register(Strings.CMD_INCREASE_FONT_SIZE, Commands.VIEW_INCREASE_FONT_SIZE, _handleIncreaseFontSize);
CommandManager.register(Strings.CMD_DECREASE_FONT_SIZE, Commands.VIEW_DECREASE_FONT_SIZE, _handleDecreaseFontSize);
CommandManager.register(Strings.CMD_RESTORE_FONT_SIZE, Commands.VIEW_RESTORE_FONT_SIZE, _handleRestoreFontSize);
CommandManager.register(Strings.CMD_SCROLL_LINE_UP, Commands.VIEW_SCROLL_LINE_UP, _handleScrollLineUp);
CommandManager.register(Strings.CMD_SCROLL_LINE_DOWN, Commands.VIEW_SCROLL_LINE_DOWN, _handleScrollLineDown);

// There are no menu items, so bind commands directly
KeyBindingManager.addBinding(Commands.VIEW_SCROLL_LINE_UP);
KeyBindingManager.addBinding(Commands.VIEW_SCROLL_LINE_DOWN);
});