WIP: Integrate the find-and-replace panel with the Julia Console

NHDaly commented 5 years ago

Integrates the Julia Console with Atom's built-in find-and-replace plugin!

Still a work in progress. Some remaining features that are probably necessary:

[x] Show the correct number of matches (currently always shows 1 -- which is arbitrary)
[x] show "No results found" if zero matches.
[x] Allow find-previous as well as find-next
[x] Cycling through multiple results on the same line

Finished features:

[x] search through the whole Console output
[x] enable/disable Regex matching
[x] enable/disable Case-sensitive search

Some of those missing features will get easier after the refactoring planned here: https://github.com/xtermjs/xterm.js/issues/1833

But still, it's not bad as-is!! It might be already at a stage where it's worth merging after we clean up the code / pare it down to just what's needed, and add unit tests...

Fixes https://github.com/JunoLab/Juno.jl/issues/212.

pfitzseb commented 5 years ago

Nice works and thanks for the contribution! :)

One major problem with reusing find-and-replace is that it only seems to work in the workspace center and not in a dock (those toggleable things at the sides). I don't think we can change that without upstream changes (which I'm not sure would be accepted).

Replicating a minimal UI for this should be fairly easy though, I think. I'd be happy to do that though if you don't feel like it ;)

NHDaly commented 5 years ago

Nice works and thanks for the contribution! :)

:D Thanks!

and not in a dock (those toggleable things at the sides).

Hmm, but does that matter for us? Is it currently possible to open the Console in a dock on the side? Can you show me how to do that? :)

Replicating a minimal UI for this should be fairly easy though, I think. I'd be happy to do that though if you don't feel like it ;)

Yeah, agreed! If you want to do that, I'd appreciate it, yeah! You're right, i guess there's not really a reason to reuse the UI, since we won't ever support the Replace part of find-and-replace.

That said, there's a fair bit of UI to include: the search term, buttons for Regex, case sensitivity, only in selection, and whole word, and finally find next and find previous.

And finally, unfortunately, i'm currently more stuck on integrating with xterm than with the find-and-replace UI. Their current search plugin gets stuck on things like finding more than one match on the same line.

pfitzseb commented 5 years ago

Hmm, but does that matter for us? Is it currently possible to open the Console in a dock on the side? Can you show me how to do that? :)

That's the default:

With the config above you can search through the atom-ink terminal but not the REPL.

That said, there's a fair bit of UI to include: the search term, and buttons for Regex, case sensitivity, only in selection, and whole word, and finally find next and find previous.

True. I might get to it tomorrow, but no promises yet.

And finally, unfortunately, i'm currently more stuck on integrating with xterm than with the find-and-replace UI. Their current search plugin gets stuck on things like finding more than one match on the same line.

Yeah, but VSCode makes do with the same API for now so I guess we can too.

NHDaly commented 5 years ago

Oh, weird! Your atom looks different than mine.. I don't have the fancy bar on the left, and when i open the Console through the Packages > Julia > Open Console, it opens in the workspace! I'll investigate what's going on with my installation...

Thanks for showing me that! Yeah you're right that the find-and-replace is currently just looking for the last active Pane. It would probably require significantly more hacking to pretend to be a Pane, and it's probably not worth it. A custom UI makes sense to me then. :) Thanks!

Yeah, but VSCode makes do with the same API for now so I guess we can too.

Oh cool, i didn't realize. Yeah, makes sense then! I think with a custom UI those above problems won't be as relevant anyway. Thanks.

pfitzseb commented 5 years ago

Ok, I've taken a stab at this (UI, keybindings and integration are done):

Would appreciate you trying this (especially since you're on a mac, right?) -- I never get keybindings right there.

NHDaly commented 5 years ago

Would appreciate you trying this (especially since you're on a mac, right?) -- I never get keybindings right there.

Yeah it works great! :) Thanks for putting this together so fast! I agree that this plays more nicely with the interface exposed by the xterm.js search plugin, and because of the Dock thing I didn't know about, I definitely agree this is a better approach. :) Thanks for putting it together! :)

These two "missing features" i have in the top comment (reproduced here) still apply, but they're due to limitations in xterm.js's search which will hopefully improve with the redesign.

Cycling through multiple results on the same line
Second search fails for same string if only occurs once (a consequence of the previous line). This also means that if you search "Doc" and then "Documentation", the second search will fail.

I think this looks great and would be good to merge any time! :) Thanks for the fast work here, @pfitzseb! ❤️❤️❤️

NHDaly commented 5 years ago

One problem to note: for me, when the regex doesn't parse, it opens the javascript developer console to show a javascript exception:

Uncaught SyntaxError: Invalid regular expression: /*j/: Nothing to repeat
    at RegExp (<anonymous>)
    at SearchHelper._findInLine (/Users/nathan.daly/Documents/atom-packages/atom-ink/node_modules/xterm/lib/addons/search/SearchHelper.js:76:31)
    at SearchHelper.findNext (/Users/nathan.daly/Documents/atom-packages/atom-ink/node_modules/xterm/lib/addons/search/SearchHelper.js:20:27)
    at findNext (/Users/nathan.daly/Documents/atom-packages/atom-ink/node_modules/xterm/lib/addons/search/search.js:10:41)
    at Terminal.terminalConstructor.findNext (/Users/nathan.daly/Documents/atom-packages/atom-ink/node_modules/xterm/lib/addons/search/search.js:23:16)
    at TerminalSearchUI.findNext (/Users/nathan.daly/Documents/atom-packages/atom-ink/lib/console2/searchui.js:51:31)
    at HTMLElement.inkTerminalFindNext (/Users/nathan.daly/Documents/atom-packages/atom-ink/lib/console2/console.js:51:25)
    at CommandRegistry.handleCommandEvent (/Applications/Atom.app/Contents/Resources/app.asar/src/command-registry.js:384:43)
    at KeymapManager.module.exports.KeymapManager.dispatchCommandEvent (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/atom-keymap/lib/keymap-manager.js:617:16)
    at KeymapManager.module.exports.KeymapManager.handleKeyboardEvent (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/atom-keymap/lib/keymap-manager.js:408:22)
    at WindowEventHandler.handleDocumentKeyEvent (/Applications/Atom.app/Contents/Resources/app.asar/src/window-event-handler.js:110:34)

Can we catch that exception and show it somehow? This is what the built-in find-and-replace package does:

pfitzseb commented 5 years ago

Good point -- fixed that in the latest commit and cleaned up the code a bit:

I think this should be good to go now.

pfitzseb commented 5 years ago

And with the new xterm.js release it's possible to find multiple items on the same line. :tada:

NHDaly commented 5 years ago

Good point -- fixed that in the latest commit and cleaned up the code a bit:

Looks beatiful! :) Thanks!

And with the new xterm.js release it's possible to find multiple items on the same line. 🎉

Haha woohoo that's awesome! :D :D Perfect timing! :)

:D Thanks again for picking this up and doing it so fast! Looking forward to it dropping! XD

pfitzseb commented 5 years ago

Alright, new release is out!

NHDaly commented 5 years ago

Looks great!! :D Thanks again! :)

JunoLab / atom-ink

WIP: Integrate the find-and-replace panel with the Julia Console #187