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

Disable PUNCTUATION_PARAGRAPH_END for current line #46

Closed
tobiasdiez opened this issue Jun 6, 2020 · 7 comments
Closed

Disable PUNCTUATION_PARAGRAPH_END for current line #46

tobiasdiez opened this issue Jun 6, 2020 · 7 comments
Assignees
Labels
1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature 3-fixed Issue resolution: Issue has been fixed on the develop branch
Milestone

Comments

@tobiasdiez
Copy link

Is your feature request related to a problem? Please describe.
If you are in the middle of writing a sentence, for example at "This is a |" (where | is the cursor), then ltex complains that there is no punctuation by underlying a. While being correct, it is distracting because one things there is a problem with the last word but its actually only because one has not yet finished the sentence.

Describe the solution you'd like
Ignore the rule PUNCTUATION_PARAGRAPH_END for the current sentence.

Describe alternatives you've considered
Disabling PUNCTUATION_PARAGRAPH_END completely, which is what I do currently.

@tobiasdiez tobiasdiez added the 1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature label Jun 6, 2020
@valentjn
Copy link
Owner

valentjn commented Jun 7, 2020

@tobiasdiez I see where you're coming from, but I wouldn't do it as you suggested:

  • Deactivating a specific rule would be language-dependent. For English, it might be PUNCTUATION_PARAGRAPH_END, for Japanese, it might be a different one. Besides, you would also have to worry about spelling diagnostics if you're in the middle of an unfinished word.
  • What is the "current sentence"? Is it the sentence the caret is currently in? Then it would be pretty annoying for me if some diagnostics vanish and pop up again when navigating through a document with the up/down arrow keys.
  • Other language linters (for programming languages) show errors at the end of the line that's being typed, too. So it's not uncommon to do this. It also doesn't bug me as much.

What I can imagine is having some kind of detection whether the user is typing (we only get text change events, so we have to determine when they stopped), and if they are, omit any diagnostics at the current caret position. The diagnostics would pop up one or two seconds after they stopped typing.

@tobiasdiez
Copy link
Author

While formulating the feature suggestion I also realized that it might be hard to implement properly.

What about combining both ideas:

  • Listen to the text change events and use it to define what is the currently edited line.
  • If the caret is still in this line, disable for that line a bunch of rules which are usually violated if a sentence is incomplete (e.g the punctuation warning).

For now I'm also happy with completely disabling this rule, so if you fell like this cannot be implemented in a nice fashion, feel free to close this suggestion.

@valentjn
Copy link
Owner

valentjn commented Jun 7, 2020

The LSP spec only supports text change events. We do not know where the caret is; we'd have to infer that from the text change events. There's microsoft/language-server-protocol#718 proposing to implement some kind of notification for that. Of course it would be possible to somehow get the info, but I'm a bit reluctant to write a workaround just tailored to VS Code, since that kind of defeats the purpose of LSP.

@valentjn valentjn added the 2-upstream Issue status: Bug is caused by some dependency, might have to wait before continuing label Jun 7, 2020
@valentjn valentjn self-assigned this Jun 10, 2020
@valentjn valentjn added this to the 6.0.0 milestone Jun 10, 2020
@valentjn
Copy link
Owner

I've implemented the following: All incremental document updates that add or delete one character are considered keystrokes that set the caret position, and any diagnostics at the caret position (and one character before) are withheld and sent to the client after two seconds.

preview

@valentjn valentjn added 3-fixed Issue resolution: Issue has been fixed on the develop branch and removed 2-upstream Issue status: Bug is caused by some dependency, might have to wait before continuing labels Jun 12, 2020
@valentjn
Copy link
Owner

Fix released in 6.0.0.

@valentjn
Copy link
Owner

valentjn commented Oct 10, 2020

The incremental document updates sporadically led to diverging documents (document in the editor vs. what LTEX thinks what the document is), i.e., the positions of the diagnostics were not matching the contents of the document anymore. I'm not sure whether the reason was messy RPC communication, or whether LTEX incorrectly applied the incremental updates. Anyways, with 7.3.0, full document updates will be used again, as correct diagnostics are too important. Unfortunately, this means that we cannot guess the position of the caret anymore, so this feature is then disabled.

If this bothers people, I suggest opening an issue for VS Code to not immediately request diagnostics while typing. I would say LTEX is behaving as expected: If VS Code requests diagnostics, it gets diagnostics.

@valentjn valentjn added 3-not-our-issue Issue resolution: This issue cannot be solved in LTeX, but in some dependency and removed 3-fixed Issue resolution: Issue has been fixed on the develop branch labels Oct 10, 2020
valentjn added a commit to valentjn/ltex-ls that referenced this issue Oct 11, 2020
@valentjn
Copy link
Owner

I've implemented a workaround to guess the position of the caret even for full document updates, so this should work again in the next release.

@valentjn valentjn added 3-fixed Issue resolution: Issue has been fixed on the develop branch and removed 3-not-our-issue Issue resolution: This issue cannot be solved in LTeX, but in some dependency labels Oct 11, 2020
me-johnomar added a commit to me-johnomar/ltex-ls that referenced this issue Jan 31, 2024
me-johnomar added a commit to me-johnomar/ltex-ls that referenced this issue Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature 3-fixed Issue resolution: Issue has been fixed on the develop branch
Projects
None yet
Development

No branches or pull requests

2 participants