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

Add support for checking programming language string literals #457

Closed
oblitum opened this issue Nov 12, 2021 · 9 comments
Closed

Add support for checking programming language string literals #457

oblitum opened this issue Nov 12, 2021 · 9 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

@oblitum
Copy link

oblitum commented Nov 12, 2021

Note: Per the contribution guidelines, deleting parts of the template or not filling in vital information may result in the issue to be immediately closed as invalid.

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
Using actual real-world examples that explain why you and many other users would benefit from the feature increases the request's chances of being implemented.

Currently I'm sporadically using LTEX for programming language comments to great effect. If it's done for comments, why not for string literals? I think it would be nice to have as an option.
.
Describe the solution you'd like
A clear and concise description of what you want to happen.

I wish an option to have it enabled/disabled for programming language string literals.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

None

Additional context
Add any other context or screenshots about the feature request here.

#350

@oblitum oblitum added the 1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature label Nov 12, 2021
@valentjn
Copy link
Owner

valentjn commented Nov 12, 2021

LTEX is primarily a grammar checker, not only a spell checker. From my experience, only few strings are actual sentences. A lot of strings are not even natural language at all (e.g., vscode-ltex's code contains many i18n keys and SHA hashes as strings), which would generate many false positives. There are already spell checkers for strings, at least for VS Code (not sure about Neovim).

On the other hand, string parsing is non-trivial to implement (e.g., character vectors in MATLAB, template strings in TypeScript, etc.). So the benefit/cost ratio is pretty small here IMO.

Can you please add actual real-world examples (programming languages, example scenarios/strings, etc. for which you would use this) to the feature request, as the template says?

@oblitum
Copy link
Author

oblitum commented Nov 12, 2021

Hi. TBH, I'm naked at such actual real-world examples, and FWIW, before posting I realized the shortcomings you're describing. I think some were referred to in #350 as well.

I just have one argument over this: LTEX is a very useful tool, and it's in my machine, it's a lost opportunity not being able to simply toggle it on at some very specific moment when I wish to proofread a document. That's a conscious choice, so I surely realize the effect of the many false positives it can bring, but when doing that I'm generally interested in some specifics of the document, LTEX even provides the feature of checking a given selection, so I imagine toggling it over some code selection to proofread the strings in it, for example.

Notice the usability here isn't to frame this feature in the same sense as it applies to officially supported filetypes, I surely wouldn't use it always enabled in programming languages, I'd only toggle it when I feel necessary, possibly toggling it off afterwards.

FWIW, I apply almost the same workflow regarding comments proofreading too. I use it on a few occasions, but it's extremely handy to have.

gif-2021-11-12-182058

@oblitum
Copy link
Author

oblitum commented Nov 12, 2021

Notice I don't mean to simply increase the amount of content being checked in programming languages, to add upon comments. Ideally, when enabling LTEX, in the referred workflow above, it would be best to be able to toggle "check comments" and "check strings" independently.

@oblitum
Copy link
Author

oblitum commented Nov 12, 2021

I realize the cost for parsing string literals, it's indeed a complicated matter. Please close it if you weight it's too much complexity for not much benefit.

@oblitum
Copy link
Author

oblitum commented Nov 12, 2021

This made me realize... if supporting many string literal types is non trivial, and given the usecase I just described, what about a feature like: "force check of selection". So you wouldn't have to bother with literals, and I would still be able to reach the tool I have, constraining it to where I wish it to apply, regardless of context (filetype supported or not, comment or not, literal or not, won't matter).

This made realize of the raw language-tool plugins I already have used. Not sure whether ltex could work like them, I didn't check.

@oblitum
Copy link
Author

oblitum commented Nov 12, 2021

Ah. Sorry the noise!

CocCommand ltex.checkSelection already fulfills this. 😅

@oblitum oblitum closed this as completed Nov 12, 2021
@oblitum
Copy link
Author

oblitum commented Nov 12, 2021

Hmmm, actually my bad, ltex.checkSelection only worked on a scratch file without a filetype (dunno why here it worked as intended). Trying it over some programming language file had no effect, even when it's toggled on for the given filetype, I got all comments checked, but CocCommand ltex.checkSelection over some string literal contents had no effect in checking it, and it actually caused the document checks to be cleared (checkSelection seems to behave as expected only in markdown, latex, etc).

@oblitum oblitum reopened this Nov 12, 2021
@valentjn valentjn self-assigned this Nov 16, 2021
@valentjn valentjn added the 3-fixed Issue resolution: Issue has been fixed on the develop branch label Nov 16, 2021
@github-actions
Copy link

This issue is now fixed on develop. The fix will be included in the next release of LTEX.

If you don't want to wait, you can try out the nightly pre-release tomorrow. Nightly pre-releases are published every morning at around 4am UTC.

@valentjn valentjn added this to the 13.1.0 milestone Nov 16, 2021
@valentjn
Copy link
Owner

Fix released in 13.1.0.

GitMensch added a commit to GitMensch/ltex that referenced this issue Mar 11, 2022
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
Archived in project
Development

No branches or pull requests

2 participants