Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump electron to latest possible version, with "native" Superstring #713

Draft
wants to merge 106 commits into
base: master
Choose a base branch
from

Conversation

mauricioszabo
Copy link
Contributor

@mauricioszabo mauricioszabo commented Sep 8, 2023

Related to: #484

Basically, the same code except that we're now using the native version of Superstring. This is to avoid slowdowns on files with too many lines.

Basically, this brings the time to a KeyDown event on files with 15k lines from 80ms (WASM Superstring) to 10ms (Native, this PR)

Currently, Electron 27.0.0

Bugs I found:

  • CTags is not context-aware
  • GitHub package is not working on this version
  • No terminal package will work (possible fix: Context-aware node-pty Spiker985/x-terminal-reloaded#41)
  • Spellchecker is also not context-aware
  • Tree-view doesn't delete files for some reason
  • Hydrogen is not working
  • Some errors on keymaps - Uncaught TypeError: Cannot read properties of undefined (reading '0')

Update on keymap error - fixed via pulsar-edit/atom-keymap@3a9a9b4

With NAPI superstring, I also found this error on line 231 of autocomplete-plus/lib/subsequence-provider.js:

Uncaught (in promise) TypeError: Cannot set property score of #<SubsequenceMatch> which has only a getter
    at bufferResultsToSuggestions (/home/mauricio/projects/pulsar_repos/autocomplete-plus/lib/subsequence-provider.js:231:27)
    at async Promise.all (index 2)
bufferResultsToSuggestions @ /home/mauricio/projects/pulsar_repos/autocomplete-plus/lib/subsequence-provider.js:231
Promise.then (async)
getSuggestionsFromProviders @ /home/mauricio/projects/pulsar_repos/autocomplete-plus/lib/autocomplete-manager.js:354
findSuggestions @ /home/mauricio/projects/pulsar_repos/autocomplete-plus/lib/autocomplete-manager.js:250
autocomplete-plus:activate @ /home/mauricio/projects/pulsar_repos/autocomplete-plus/lib/autocomplete-manager.js:223
handleCommandEvent @ /home/mauricio/projects/pulsar_repos/pulsar/src/command-registry.js:405
dispatchCommandEvent @ /home/mauricio/projects/pulsar_repos/pulsar/node_modules/atom-keymap/src/keymap-manager.js:926
handleKeyboardEvent @ /home/mauricio/projects/pulsar_repos/pulsar/node_modules/atom-keymap/src/keymap-manager.js:671
handleDocumentKeyEvent @ /home/mauricio/projects/pulsar_repos/pulsar/src/window-event-handler.js:153

This probably can be solved by not re-setting the score at the builtin provider, or making a clone (which we probably should be doing anyway)

mauricioszabo and others added 30 commits April 11, 2023 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant