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 incremental checking based on built-in sentence navigation #24

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

tpeacock19
Copy link
Contributor

This allows us to send only partial chunks of text to the server
instead of sending the entire buffer for each modification to the
buffer.

@tpeacock19 tpeacock19 linked an issue Mar 2, 2024 that may be closed by this pull request
@tpeacock19 tpeacock19 force-pushed the feat/incremental-checking branch 3 times, most recently from df18bba to 18ed07e Compare March 4, 2024 07:59
@tpeacock19
Copy link
Contributor Author

@jcs090218 I think this is finally in a good place for a real review. Whenever you have time can you take a look and try it out? There are still many areas I wish to refactor/optimize; however, it'll be better to get more eyes on it.

Copy link
Member

@jcs090218 jcs090218 left a comment

Choose a reason for hiding this comment

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

There is a compile warning:

flymake-languagetool.el:443:16: Warning: reference to free variable ‘forward-sentence-function’

See log https://github.com/emacs-languagetool/flymake-languagetool/actions/runs/8148448182/job/22271266671?pr=24#step:5:64.

README.md Outdated Show resolved Hide resolved
Copy link
Member

@jcs090218 jcs090218 left a comment

Choose a reason for hiding this comment

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

Overall, LGTM!

I didn't check the functionalities, but you have been very responsive, so I think I can trust you. Feel free to merge it when it's ready! :)

@tpeacock19 tpeacock19 force-pushed the feat/incremental-checking branch from 220bbbd to b8e871f Compare March 7, 2024 04:26
@tpeacock19 tpeacock19 linked an issue Mar 7, 2024 that may be closed by this pull request
@tpeacock19
Copy link
Contributor Author

@jcs090218 Is there any way to get the Windows-latest snapshot test to actually run? I have seen that it always fails at the eask clean all command.

@jcs090218
Copy link
Member

No, unfortunately. For more info, see emacs-eask/cli#224.

@tpeacock19 tpeacock19 force-pushed the feat/incremental-checking branch from 888f4bd to bc6afc5 Compare April 2, 2024 04:14
@tpeacock19
Copy link
Contributor Author

@jcs090218 I was going to merge this PR, but I'm not certain why all the macOS tests are failing - there is not much information in logs. Whenever you have time, can you take a look?

@jcs090218
Copy link
Member

I've re-run the test. It looks good now! ;)

@jcubic
Copy link

jcubic commented Nov 8, 2024

Any progress on this? Will it be merged? It looks like only one Windows test is failing.

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.

Incremental Checks Longer files throws an error
3 participants