ejgallego / coq-lsp

Visual Studio Code Extension and Language Server Protocol for Coq
GNU Lesser General Public License v2.1
137 stars 31 forks source link

[Bug?] Incorrect activation events #737

Closed kands-code closed 1 month ago

kands-code commented 1 month ago

Describe the bug

For most plugins in vscode, specific language plugins, such as Java, or Python, the activation events is generally onLanguage:xxx, where xxx represents this language, such as Java:

Screenshot of plugin interface ![image](https://github.com/ejgallego/coq-lsp/assets/59006329/64758ad4-8729-4b2b-b573-7257273d4fae)

including VsCoq:

Screenshot of plugin interface ![image](https://github.com/ejgallego/coq-lsp/assets/59006329/91263966-5274-48a9-8dbb-5fa2be5c771c)

But the activation events of CoqLSP are markdown and latex:

Screenshot of plugin interface ![image](https://github.com/ejgallego/coq-lsp/assets/59006329/27683211-7302-4522-b09b-53d4d2893edb)

As a result, in some projects not related to Coq, there may be a README.md in the project, and then CoqLSP is activated.

To Reproduce

Steps to reproduce the behavior:

  1. Go to 'Extensions'
  2. Click on 'Coq LSP'
  3. Scroll down to 'Activation Events'

Expected behavior

The activation event of the plugin should be onLanguage:coq

Desktop (please complete the following information):

kands-code commented 1 month ago

If you want to add support for "literate coq", I think you can refer to Agda's approach.

{
  // ...
  "contributes": {
    // ...
    "languages": [
      {
        "id": "agda",
        "extensions": [".agda"],
        "aliases": ["Agda"],
        "configuration": "./language-configuration.json",
        "icon": {
          "dark": "./asset/dark.png",
          "light": "./asset/light.png"
        }
      },
      {
        "id": "lagda-md",
        "extensions": [".lagda.md"],
        "aliases": ["Literate Agda (markdown)"],
        "configuration": "./language-configuration.json",
        "icon": {
          "dark": "./asset/dark.png",
          "light": "./asset/light.png"
        }
      },
      {
        "id": "lagda-tex",
        "extensions": [".lagda.tex", ".lagda"],
        "aliases": ["Literate Agda (TeX)"],
        "configuration": "./language-configuration.json",
        "icon": {
          "dark": "./asset/dark.png",
          "light": "./asset/light.png"
        }
      },
      {
        "id": "lagda-rst",
        "extensions": [".lagda.rst"],
        "aliases": ["Literate Agda (reStructuredText)"],
        "configuration": "./language-configuration.json",
        "icon": {
          "dark": "./asset/dark.png",
          "light": "./asset/light.png"
        }
      }
    ]
    // ...
  }
  // ...
}

For example:

{
  // ...
  "contributes": {
    // ...
    "languages": [
      {
        "id": "coq",
        "extensions": [".v"],
        "aliases": ["Coq"],
        "configuration": "./language-configuration.json"
      },
      {
        "id": "coq-md",
        "extensions": [".mv", ".lcoq.md"],
        "aliases": ["Literate Coq (markdown)"],
        "configuration": "./language-configuration.json"
      },
      {
        "id": "coq-tex",
        "extensions": [".lv", ".lcoq.tex"],
        "aliases": ["Literate Coq (TeX)"],
        "configuration": "./language-configuration.json"
      }
    ]
    // ...
  }
  // ...
}

However, I don't know the details of the implementation, so it's just a suggestion.

ejgallego commented 1 month ago

Hi @kands-code ,

thanks for the report, the code you quote is for 0.1.9, so indeed the behavior will differ greatly from 0.1.8.

Activaton is a bit complex, as indeed, coq-lsp does want to be activated for some buffers, including those that are literate markdown or LaTeX, but only when the extension is .mv or .lv / .v.tex.

A few comments:

Thanks for the Agda example, we used this approach in the past, however we had the problem that regular markdown editing facilities are not available, are they in Agda case?

But indeed, in the Agda code, as the extension does contribute the language, the onLanguage event is not needed, but automatically inferred.

See https://github.com/microsoft/vscode/issues/198295 for discussion about this topic.

Any help with this is much appreciated.

ejgallego commented 1 month ago

Things seems to work fine in 0.1.9, except for a little detail:

We will release a 0.1.9.1 bugfix ASAP (also to tweak a few other things).

ejgallego commented 1 month ago

@kands-code , the 0.1.10 version of the extension should behave much better, thanks for the report and testing.

Don't hesitate to reopen if the coq-lsp extension gets in the way again.