-
Notifications
You must be signed in to change notification settings - Fork 116
Mark autocomplete #377
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
Open
platers
wants to merge
25
commits into
WuTheFWasThat:master
Choose a base branch
from
platers:autocomplete
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Mark autocomplete #377
Changes from 19 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
d28e881
switch mark from word hook to line hook
platers 3c06451
[[]] to mark
platers 6e27c1e
allow spaces in marks
platers ac601a1
refactor marks
platers 50a3771
fix gm for [[]] marks
platers 32e04d9
refactor and update mark tests
platers d151049
update mark instructions
platers 7758ef4
lint and cleanup
platers 7f917ac
M to set a mark with row contents
platers acbbcbb
display mark autocompletions
platers bfea1b5
autocomplete functional
platers 0497959
autocomplete position on mark
platers 71b377e
small mark cleanup
platers 4306793
merge
platers 6ad74a8
lint
platers a5e71c3
fixed autocomplete displaying index instead of match
platers 6ec1e9f
autocomplete always on top of other elements
platers 00dd20e
merge master
platers a81a3a9
Merge branch 'master' into autocomplete
platers 8660195
remove session from props
platers 1cc0eda
mark search use includes, not prefix
platers cd1928c
search marks case insensitive
platers d8907a8
refactor autocomplete to use mode
platers 03b17f2
update tests and lint
platers 9f2feff
autocomplete tests
platers File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
might be better to not have the Session abstraction here and just pass in something like
renderLineAnnotationHook?: (info: { lineData: .., column: ..., cursors: ...}) => Array<React.ReactNode>,
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.
I got it working that way, but I had to bind the session to the function. Does that defeat the purpose? Genuinely curious exactly why this way is cleaner.
Uh oh!
There was an error while loading. Please reload this page.
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.
yeah, fine to bind the session to the function from the caller. i don't know if it really matters, was just going broadly by the principle of requiring less abstractions in interfaces :)
for example if you wrote a unit test for line.tsx it would be maybe annoying if you had to construct a session object to test
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.
makes sense, thanks for explaining!