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

Implement editing, find/replace in streaming model #316

Merged
merged 20 commits into from
Nov 29, 2021
Merged

Conversation

connor4312
Copy link
Member

This implements editing, include find and replace. Undo/redo doesn't work, I'll toss that in tomorrow morning. After this I'll swap to some of my other iteration work.

@connor4312 connor4312 self-assigned this Nov 17, 2021
Copy link
Member

@lramos15 lramos15 left a comment

Choose a reason for hiding this comment

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

👍 . Some comments on those larger functions :). Don't forget to fix the bugs we went over

@connor4312
Copy link
Member Author

connor4312 commented Nov 20, 2021

Fixed a bunch of those, the remainder I will take care of when I return Tuesday

  • length change in modification shows on wrong change (might have fixed already, need to confirm)
  • find/replace back and forth with different lengths breaks
  • don't change selected arrow on hover
  • expansion arrow on find/replace widget
  • show error state in find instead of diallowing keypresses
  • binary search seems to find wrong
  • confirm progress indicator, regex with . not working
  • drop opId
  • make sure focus is synced to browser focus
  • tab should focus the byte in the text side
  • check out go to offset / quickpick bugs
  • return focus on escape from find widget
  • delete editsequal
  • rename currentEditor to something else
  • reimplement "??"

The quickpick bug we hit is an easy repro and captured in microsoft/vscode#137558

@lramos15 lramos15 self-requested a review November 22, 2021 21:00
@connor4312
Copy link
Member Author

This should be good to go now. I'll write up a TPI over holiday.

Copy link
Member

@lramos15 lramos15 left a comment

Choose a reason for hiding this comment

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

👍

@lramos15 lramos15 merged commit f91ed84 into streaming-2 Nov 29, 2021
@lramos15 lramos15 deleted the streaming-3 branch November 29, 2021 18:02
lramos15 pushed a commit that referenced this pull request Nov 29, 2021
* fixup! comments

* refactor: updates to webview side of things

* initial working bytes display

* data display and basic selection

* get selection more up to snuff

* finish data selection and display side

* refactor: use custom state for selection to improve performance

* tweaks

* Implement editing, find/replace in streaming model (#316)

* wip

* fixup, get replacement editing working

* unsaved ranges and go to offset

* chore: remove dead code

* feat: implement virtual scroll

* wip

* refactor: add comments and clean up math

* find/replace

* feat: undo/redo

* fix: focus issues with go to offset quickpick

* fix: focus back to the byte when hiding find widget

* fix: binary in search, cap results by default for perf, progress

* fix: don't change selected toolbar background on hover

* fix: issues with length changes and large find/replace

* feat: add replace toggle

* feat: show better validation for find widget errors

* feat: wip on string search with placeholders

This approach will not work, it's wildcards and not placeholders. But committing it if we need it in the future...

* feat: support placeholders in search

* feat: persist webview state information

* fix: make revert work
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.

2 participants