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

Address #135 #137

Merged
merged 12 commits into from Mar 3, 2020
Merged

Address #135 #137

merged 12 commits into from Mar 3, 2020

Conversation

ice1000
Copy link
Collaborator

@ice1000 ice1000 commented Mar 1, 2020

Fixes #135. Screenshot will be added later

@ice1000 ice1000 requested review from sxhya and valis Mar 1, 2020
@ice1000
Copy link
Collaborator Author

ice1000 commented Mar 1, 2020

This should be merged after #134 because it's based on it.

Copy link
Collaborator Author

@ice1000 ice1000 left a comment

Some comments

abstract fun isApplicableTo(element: T, caretOffset: Int, editor: Editor?): Boolean
abstract fun isApplicableTo(element: T, caretOffset: Int, editor: Editor): Boolean

abstract fun applyTo(element: T, project: Project?, editor: Editor?)
abstract fun applyTo(element: T, project: Project, editor: Editor)
Copy link
Collaborator Author

@ice1000 ice1000 Mar 1, 2020

Choose a reason for hiding this comment

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

I made these not-null because these arguments are never null when these methods are invoked.
By doing this, some null-checks are removed for free.

Copy link
Collaborator Author

@ice1000 ice1000 Mar 1, 2020

Choose a reason for hiding this comment

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

@ice1000 ice1000 self-assigned this Mar 1, 2020
@ice1000 ice1000 added the subexpr label Mar 1, 2020
@ice1000
Copy link
Collaborator Author

ice1000 commented Mar 1, 2020

a

@valis valis merged commit bbb8417 into master Mar 3, 2020
1 check failed
@ice1000 ice1000 deleted the replace-norm branch Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants