Add LSP-nimlangserver

[AmjadHD]

https://github.com/sublimelsp/LSP-nimlangserver

[STReviewBot]

Automated testing result: ERROR

Repo link: LSP-nimlangserver
Results help

Packages added:
  - LSP-nimlangserver

Processing package "LSP-nimlangserver"
  - ERROR: No valid semver tags found at https://github.com/sublimelsp/LSP-nimlangserver/tags for the package "LSP-nimlangserver".

[STReviewBot]

Automated testing result: ERROR

Repo link: LSP-nimlangserver
Results help

Packages added:
  - LSP-nimlangserver

Processing package "LSP-nimlangserver"
  - ERROR: 'package-metadata.json' is supposed to be automatically generated by Package Control during installation
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 32, Column: 8
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 34, Column: 10
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 36, Column: 10
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 42, Column: 23
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 42, Column: 56
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 93, Column: 46
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 143, Column: 40

[predragnikolic]

I've just one required change. Remove package-metadata.json. (see https://packagecontrol.io/docs/submitting_a_package#Step_5)

Here are some notes and observations:

#1
This line return {"nim": (("nim",), (path[9:path.rfind('.')],))} could be a bit simplified to avoid 9:path.rfind('.')
https://github.com/sublimelsp/LSP-nimlangserver/blob/359f197ad13b9ef5cc054d4bd5353d6588881c02/plugin.py#L71C1-L73C63

#2
Indentation can be fixed:
https://github.com/sublimelsp/LSP-nimlangserver/blob/359f197ad13b9ef5cc054d4bd5353d6588881c02/LSP-nimlangserver.sublime-settings#L4

#3
You seem to support the system installed nim binary and the plugin managed nim binary.
While it makes sense to support both,
keep in mind that if the user has a outdated server version installed locally,
the sublime-package.json schema might give the wrong suggestions to the user.

I do not have a solution to suggest, except suggesting that you only have a managed version.
And if someone wants to manage their own server they can configure a LSP client configuration in LSP.sublime-settings.
... again I do not know what is right, this was just a note

#4
It would be good if the on_pre_start was be removed.
It looks misused. if you read the docs comments, you will note that it is used only for adjustments, not to forbid the server from starting.

  """
        Callback invoked just before the language server subprocess is started. This is the place to do last-minute
        adjustments to your "command" or "init_options" in the passed-in "configuration" argument, or change the
        order of the workspace folders. You can also choose to return a custom working directory, but consider that a
        language server should not care about the working directory.

https://github.com/sublimelsp/LSP-nimlangserver/blob/359f197ad13b9ef5cc054d4bd5353d6588881c02/plugin.py#L179-L188

You might want to look at:

    def can_start(cls, window: sublime.Window, initiating_view: sublime.View,
                  workspace_folders: List[WorkspaceFolder], configuration: ClientConfig) -> Optional[str]:
        """
        Determines ability to start. This is called after needs_update_or_installation and after install_or_update.
        So you may assume that if you're managing your server binary, then it is already installed when this
        classmethod is called.

but again it would be good to avoid can_start or on_pre_start if possible, just to have less code.

[predragnikolic]

I am on Linux,
I cloned the repo locally, and deleted package-metadata.json,
I installed NimLime. I do not have nim in my system path,
so I set "binary": "",
(btw I would suggest the default for "binary" to be an empty string instead of nimlangserver??)

	// The nimlangserver binary to use.
    // if set to `""`, the server is auto installed and updated.
-    "binary": "nimlangserver",
+    "binary": "",

but still nothing happens,
nothing is installed in the Package Storage folder.

[AmjadHD]

1. There are two packages that provide syntaxes for Nim, I want it to work with either.
2. Done.
3. The plugin checks for updates, and prompts the user to upgrade if they're using the the system version. The user can either accept and the plugin switches to managed mode, or cancel in which case they can update manually.
4. How else am I going to set the command ? The check is not really meant to stop the server, it's to keep Pyright happy, Also See https://github.com/sublimelsp/LSP-clangd/blob/f7bbdf23525c0657ad6abdc0eac4e4a54695900d/plugin.py#L173-L175.

Also regarding can_start, I expect the message strings returned when the server can't be started to be shown in an error dialog, but It doesn't.

nothing is installed in the Package Storage folder.

Do you see anything in the console ?

[predragnikolic]

1. There are two packages that provide syntaxes for Nim, I want it to work with either.

My main concern someone will ask why the magic number 9 is in path[9:path.rfind('.')],)). I didn't know what it did, it strips Packages from the string... which was not that obvious when I first saw that peace of code...

4. ... it's to keep Pyright happy

👍

Do you see anything in the console ?

I been running LSP python 3.8, that is the reason why LSP-nimlangserver didn't installed the server in Package Storage.
But when I switched back to 3.3, it installed the server, but I still get a message in the status bar "Nim is not in PATH."

I have "binary": "" in my settings and I do not have nim in the system path.
so I end up in this if here https://github.com/sublimelsp/LSP-nimlangserver/blob/71aa0f17f7543bd4b600d824b041a2fa4fecf06f/plugin.py#L177

after installing nim, it works.

I expect the message strings returned when the server can't be started to be shown in an error dialog

it will be displayed in the status bar.

[predragnikolic]

Note, you still need to tag a new release in order to fix package-metadata.json.

[AmjadHD]

I published a release.

[AmjadHD]

My main concern someone will ask why the magic number 9 is in path[9:path.rfind('.')],)). I didn't know what it did

Python, afaik, doesn't do constant folding. I sometimes have this bad habit of premature optimization 🙂.

[STReviewBot]

Automated testing result: WARNING

Repo link: LSP-nimlangserver
Results help

Packages added:
  - LSP-nimlangserver

Processing package "LSP-nimlangserver"
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 32, Column: 8
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 34, Column: 10
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 36, Column: 10
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 42, Column: 23
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 42, Column: 56
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 93, Column: 46
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: plugin.py
    - Line: 143, Column: 40

[predragnikolic]

Thanks :)