-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Enhance status bar to show length of selection #4579
Conversation
line, line(s) or partial lines if selection spans more than one). Hoist out \t converion logic from EditorgetCursorPos(). Add unit tests for it.
@@ -156,6 +156,8 @@ define({ | |||
* StatusBar strings | |||
*/ | |||
"STATUSBAR_CURSOR_POSITION" : "Line {0}, Column {1}", | |||
"STATUSBAR_CH_SELECTION_LEN" : " \u2014 Selected {0} column(s)", | |||
"STATUSBAR_LINE_SELECTION_LEN" : " \u2014 Selected {0} line(s)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two strings have two things not consistent with total lines strings on line 167 and 168
"STATUSBAR_LINE_COUNT_SINGULAR" : "\u2014 {0} Line",
"STATUSBAR_LINE_COUNT_PLURAL" : "\u2014 {0} Lines",
Total line count strings do not have a preceding space and we're using separate string for singular and plural line count. So when you have some selection, Brackets is going to show the inconsistency with Selected n line(s) -- nnn lines
Regarding the leading space I wonder we should just concatenate the hard-coded string --
in the code rather than having the separator character and spaces in the localizable strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The separator is already used in STATUSBAR_LINE_COUNT_*, and I'm not sure if separators are ever locale-specific (quotation marks certainly are), so to be safe I'd prefer to leave it as-is for now.
I will fix the pluralization issue though.
Done first review. |
…ar - never use the indefinite "(s)" suffix anymore.
@RaymondLim Fix pushed. Do you think you could squeeze this in today? That way it can land before string freeze. (My fault if not, though -- sorry for the delay in responding!) |
@@ -49,13 +49,17 @@ define(function (require, exports, module) { | |||
$indentWidthInput; | |||
|
|||
|
|||
function _formatCountable(number, singularStr, pluralStr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a nice StringUtils function. It is also missing the JSDocs. But lots of functions in this file have the same problem. Could be fixed later.
Looks good! Merging. |
Enhance status bar to show length of selection
Show length of selection when one is present -- number of cols if within a single line, number of lines (or partial lines) if selection spans more than one.
Hoists out "\t" conversion logic from Editor.getCursorPos(). Add unit tests for it.
If we're really not into putting this in core, this seems doable as an extension too... just not nearly as cleanly.