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

feat: delete "sentence" shortcut #32

Merged
merged 7 commits into from
Apr 11, 2023

Conversation

Xazin
Copy link
Collaborator

@Xazin Xazin commented Mar 25, 2023

Closes: #4

Need to check if the commands are properly setup for Linux and Windows!

@Xazin Xazin force-pushed the feat/delete-sentence-left-cursor branch from f17b6cb to 0a5b3a1 Compare March 25, 2023 17:33
@Xazin Xazin force-pushed the feat/delete-sentence-left-cursor branch from 0a5b3a1 to 7d47401 Compare March 25, 2023 19:11
@codecov
Copy link

codecov bot commented Mar 25, 2023

Codecov Report

Merging #32 (f6b5597) into main (6e72e58) will increase coverage by 0.97%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##             main      #32      +/-   ##
==========================================
+ Coverage   81.68%   82.65%   +0.97%     
==========================================
  Files         124      124              
  Lines        7178     7184       +6     
==========================================
+ Hits         5863     5938      +75     
+ Misses       1315     1246      -69     
Impacted Files Coverage Δ
lib/src/core/document/text_delta.dart 95.66% <ø> (ø)
...nternal_key_event_handlers/arrow_keys_handler.dart 98.32% <91.30%> (+17.66%) ⬆️
...rvice/shortcut_event/built_in_shortcut_events.dart 98.16% <100.00%> (+0.03%) ⬆️

... and 11 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@LucasXu0
Copy link
Collaborator

LucasXu0 commented Apr 6, 2023

Hey, @Xazin. Can you add more tests for the missing cover line?

@Xazin
Copy link
Collaborator Author

Xazin commented Apr 6, 2023

I will look at it tonight 👍

@LucasXu0
Copy link
Collaborator

There are some lines of code that don't have test coverage. I think it would be helpful to have test coverage for all of the lines. Would you be willing to work on that?

@Xazin
Copy link
Collaborator Author

Xazin commented Apr 10, 2023

There are some lines of code that don't have test coverage. I think it would be helpful to have test coverage for all of the lines. Would you be willing to work on that?

Yes for sure.

I ended up busy this weekend with another project. But I'm back looking at this, will make things a bit prettier and improve test coverage for all of arrow_keys_handler.dart.

@Xazin Xazin force-pushed the feat/delete-sentence-left-cursor branch from 260ed4f to 2cc9be6 Compare April 10, 2023 10:52
@Xazin Xazin force-pushed the feat/delete-sentence-left-cursor branch from 2cc9be6 to 1b014c2 Compare April 10, 2023 10:57
@Xazin
Copy link
Collaborator Author

Xazin commented Apr 10, 2023

@LucasXu0 Ready for review

Made some changes to the arrow keys handler file, but the logic is and should be the same.

The file should have 98% coverage now, few lines not tested, not worth the effort imo atm.

Co-authored-by: Mathias Mogensen <42929161+Xazin@users.noreply.github.com>
@LucasXu0
Copy link
Collaborator

LGTM.

@LucasXu0 LucasXu0 merged commit 47ed51a into AppFlowy-IO:main Apr 11, 2023
Xazin added a commit to Xazin/appflowy-editor that referenced this pull request Apr 13, 2023
* feat: delete sentence shortcut

Closes: AppFlowy-IO#4

* fix: error in shortcut_event_test.dart

* fix: prettify and add missing test

* test: improve coverage for arrow_keys_handler.dart

* chore: Apply suggestions from code review

Co-authored-by: Mathias Mogensen <42929161+Xazin@users.noreply.github.com>

---------

Co-authored-by: Lucas.Xu <lucas.xu@appflowy.io>
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.

cmd + backspace deletes word instead of all text to the beginning of the line
2 participants