Refactor nimsuggest into multiple files

[faldor20]

This is an attempt to make maintenance on nimsuggest more possible.

I have been using nim and wanted to try to improve the tooling, but immediately bounced off trying to do anything in nimsuggest.
A monster single file approaching 1000 lines with almost no comments or docs was just completely inscrutable.

I've broken out each part into logical chunks and added a little documentation.

I've marked this as a draft because I understand there will likely be feedback and I'd also like to extend the refactoring into generally cleaning up the code as well.

Would you like that to be broken into a second PR?
Eg: one for the split, then a second for the code changes.

I'd like to follow this up with some docs on how suggestion and nimsuggest work in conjunction with the compiler. Though I understand that might not be worthwhile what with IC coming and potentially significantly changing that. Thoughts?

[Araq]

1. Splitting up nimsuggest into more file is a good idea.
2. How you did it, not so much. Now there is v3 and consts without much logic behind it.
3. Alien coding style, use spaces around binary operators, don't use single line import, instead group them properly. The ideal is to make import statements disappear as they are for the compiler, not for human beings.
4. Wrong order: You don't "refactor" code because you don't understand it. You refactor it after you understood it well enough. Ideally your understanding is objectively judged by a pull request that did a feature addition or a bugfix.

[faldor20]

Whilst I'm no expert, I feel like I understand this part of the codebase fairly well. The nimsuggest v3 code seems completely seperate from the v2 v1 and v0 code. I think it makes sense to cleanly separate them, I wish the other versions were easier to extricate from each other.

I completely agree about the consts file, it's a mess, the EOF const needs to be accessed a few places and the help text needs somewhere to live . I was thinking maybe move the help stuff to helpTexts.nim and the EOF into the utils.nim file. Thoughts?

Yeah, sorry about the formatting, just used to using autoformatters and not thinking about it. Easy fix.
I broke up the imports for ease of refactoring, so nimsuggest could easily tell me the unused ones. I'll recombine them before this PR is done.

I was hoping to make a some small improvements to nimsuggest and nimlangserver/nimlsp, (I like IDE tooling and have done some work there in other languages) but when I came to do so i was confronted with that undocumented, uncommented tangle. I mean the v3 code was just squashed in between "#v3 start" and "#v3 end" comments. I didn't think adding to the mess was going to be a help.

Anyway, all of this is why I marked it as a draft. I wanted to get something rough in front of you all to check if it's even wanted, and if you had any initial objections. I just don't want to do work that nobody wants.

[Araq]

Nice work, but the CI must be green or at least contain unrelated errors (I haven't checked!)

[faldor20]

Thankyou :)
This last push should fix all the CI errors. I've finally nailed down the tests and it was just the exports causing packages with it as a dependency to fail.

[faldor20]

Before this gets merged there are a couple of things I want to highlight:

1. I switched the cmd parsing to using the compilers parser which required removing the case-insensitivity of commands
2. I separated the parsing and that parsing returns early when the command is not part of the standard set (quit, debug, terse) . Because it returns early the CommandData object won't be filled out in those cases.
   Because the toggle command doesn't call quit() as far as i can tell it will still try to run execute() with that mostly empty CommandData object.
   Should toggle() calls be followed by quit()? Does this matter considering these commands don't need to trigger a recompile.

[Araq]

I switched the cmd parsing to using the compilers parser which required removing the case-insensitivity of commands

What does that mean and why was this required?

Should toggle() calls be followed by quit()? Does this matter considering these commands don't need to trigger a recompile.

Cannot remember what toggle does.

[faldor20]

I saw that the compiler has it's own code for converting strings to ideCmd types.
nimsuggest duplicated this code and added a few extra commands. To reduce the pointless copying I just added the missing commands to the compilers parser.

[faldor20]

I saw that the compiler has it's own code for converting strings to ideCmd types.
nimsuggest duplicated this code and added a few extra commands. To reduce the pointless copying I just added the missing commands to the compilers parser.

[faldor20]

I call the parseIdeCmd function from the parser code instead of re-implementing it. However nimsuggest still has some extra commands that won't be parsed by the compilers parser. In that case we return those early, so they can be dealt with.

[faldor20]

This is the old re-implementation of the compilers parsing code. As you can see it is case-insensitive while the compilers is not

[faldor20]

As you can see we re-parse the cmdString if the command isn't on the list the compiler knows about to find the nimsuggest specific messages that aren't any kind of ideCmd

This is where we use the toggle command which turns debug/ terse off. I believe this should likely trigger an early return. There isn't really any reason to try to execute the rest of the code if the command is a toggle.