#2308 Go-to only numbers fix #2370
Conversation
…sor and center it. Otherwise log an error and show the console
|
Awesome, thanks for the PR! I'm hesitant to open the console on an error, but should be able to take a look at everything over the weekend. Worst case we leave the console in whatever state the user had it in and log the less verbose error. Edit: I haven't forgotten about this... I've realized how little time I have for LT over the weekend. Plan to get to it this week... ping me if you don't hear back by 5/31. |
|
Ouuu. I didn't know that there is such thing as "notifos". I find notifos a better approach here than writing it to the console, as I think console should be used for more "important" things. |
src/lt/objs/find.cljs
Outdated
| l))}) | ||
| (editor/center-cursor cur))))}) | ||
| (let [line (if-not (number? l) | ||
| (js/parseInt l) |
There was a problem hiding this comment.
parseInt(string, radix)
It is recommended to add the base for the following reasons:
If radix is undefined or 0 (or absent), JavaScript assumes the following:
- If the input string begins with "0x" or "0X", radix is 16 (hexadecimal) and the remainder of the string is parsed.
- If the input string begins with "0", radix is eight (octal) or 10 (decimal). Exactly which radix is chosen is implementation-dependent. ECMAScript 5 specifies that 10 (decimal) is used, but not all browsers support this yet. For this reason always specify a radix when using parseInt.
- If the input string begins with any other value, the radix is 10 (decimal).
There was a problem hiding this comment.
Since it was not used before, I didn't even think of specifying it
There was a problem hiding this comment.
I wasn't aware of this either... If you could add that radix parameter of 10 just to be safe, that would be great!
src/lt/objs/find.cljs
Outdated
| (let [cur (pool/last-active)] | ||
| (editor/move-cursor cur {:ch 0 :line (dec line)}) | ||
| (editor/center-cursor cur)) | ||
| (notifos/set-msg! (str "Line " l " is not a positive integer > 0") {:class "error"}))))}) |
There was a problem hiding this comment.
For clarity, perhaps change the message to be "Line number '" l "' must be a positive integer".
Currently, if the input is blank we get "Line is not a positive integer >0" and if the input was "cat" we would get "Line cat is not a positive integer > 0".
They would look like the following with the suggested message:
"Line number '' must be a positive integer""Line number 'cat' must be a positive integer"
|
This looks great @denis631! Notifos seems like a perfect fit here. Just the two minor changes and we should be all set. |
Checking whether or not the line is a positive integer if yes then move cursor and center it. Otherwise log an error and show the console