🏗 Split CompletionItemProvider into smaller modules #905
Conversation
|
Some work still required before merge I think:
|
Lemmingh
added
Area: Auto completion
Code is just moved from the the main completion function. No functional changes Function is async and has a Cancellation token, though the later could likely be better made use of. I've duplicated the lineTextBefore variable within the function rather than passing it in. This is less efficient just now but I think it will be cleaner once all completion functions are separated out. Eventually I don't think it will be needed within the function at all.
code is just moved from main completion function. No functional changes.
Just moved code from main completion function. No functional changes. lineAfterTest variable also moved in to this function as it is the only place it is used.
Code just moved from main function No functional changes.
No functional changes, just tidying code.
required renaming of previous `mathCompletion` variable to `mathCompletionItems` Just moving code from main to new function. No functional change. Consider moving `mathCompletionItems` into new function in future
|
This repo enforces squash merge and linear history for mainline now. So, after your PR got merged, you need to discard the old work branch and create a new one, or perform equivalent operations. There are many approaches. Thanks. |
Naming needs further discussion.
|
I was wondering about the LaTeX/KaTeX stuff. Should it be treated differently than the other completion parts. I mean should it be treated like it's an extension. In that way we plan for other CM "extensions" in future. Maybe that's more for discussion when we get to the longer term design. |
Matches is type string array, The line `matches = lineTextBefore.match(/\\+$/);` seems to be old code and is not doing anything of use. It is repeated a few lines further down in the if statement for maths completion.
Using named key-value pair for code clarity.
Previous capture relied on counting number of backslashes after matching a regex and performing modular arithmetic to check for an odd number.
This commit is to change the latexCmd regex to a single line which says:
`/(^|[^\\])([\\]{2})*\\$/`
```
IF ( ( (on new line) OR (following a non-backslash char) )
AND (0 or more pairs of backslashes)
AND a single backslash )
THEN there are an odd number of backslashes.
```
The benefit of this is to better align with the other regex's used in the if-else statement. also removes need for `matches` variable.
|
Please do some extra testing on 43ab0ea which updated the latex "ends in odd number of backslashes" if-statement to a single line regex and removed the need for the modulo operation. Latex is not something I use really, but I think my one-liner works to capture what is required. Also feel free to do some renaming as you see fit. |
|
Thanks @gavbarnett. Looks we are in the right direction 👍 (although I didn't test it). And sorry that I only have limited time for this extension in the following weeks. |
Lemmingh
changed the title
Stepping stone to resolve #896
No functional changes, purely structural.