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

Various improvements #36

Open
panoply opened this issue Dec 8, 2023 · 6 comments
Open

Various improvements #36

panoply opened this issue Dec 8, 2023 · 6 comments

Comments

@panoply
Copy link

panoply commented Dec 8, 2023

Historically, the Fauna team has ignored my questions pertaining to contributions which I'll begin by prefacing:

The reason for referencing these is because ignoring developers requests reflects badly, especially when they are wanting to assist and help. This will be the last time I reach out or bother trying, so it is my hope that some form of communication is had.


This extension is a great starting point but it is definitely lacking in a lot of areas. Firstly, the grammars. Token captures are limited and the vast majority of syntactics of importance are defaulting to source.fql and not asserting the correct name refs. Themes are unable to differentiate between structures given the elementary approach taken on grammars.

fql``

Because Fauna has now rolled out FQL in fql`` template literals, I'm curious as to why the extension is not supporting injections here given the importance literals now apply. The fql language grammar is already passed, an injection grammar can be leveraged. This is a basic and easy to support, e.g:

{
      "name": "string.js.taggedTemplate.literal.js",
      "contentName": "meta.embedded.block.fql",
      "begin": "(\\bfql\\b)(`)",
      "beginCaptures": {
        "1": {
          "name": "entity.name.function.tagged-template.js"
        },
        "2": {
          "name": "punctuation.definition.string.template.begin.js"
        }
      },
      "end": "(`)",
      "patterns": [
        {
          "include": "source.fql"
        },
        {
          "match": "."
        }
      ]
    }

In addition, ensure injectionSelector does not conflict with something like:

{
  "injectionSelector": "L:source.js -comment -(string - meta.embedded), L:source.jsx -comment -(string - meta.embedded), L:source.js.jsx -comment -(string - meta.embedded), L:source.ts -comment -(string - meta.embedded), L:source.tsx -comment -(string - meta.embedded)"
}

From here, you simply need to reference in contributions via package.json

 {
        "injectTo": [
          "source.js",
          "source.js.jsx",
          "source.jsx",
          "source.ts",
          "source.tsx",
          "text.html.basic",
          "text.html.derivative",
          "text.html.markdown"
        ],
        "scopeName": "<SCOPE>",
        "path": "./syntax/<NAME-OF-GRAMMAR>.tmLanguage.json",
        "embeddedLanguages": {
          "meta.embedded.block.fql": "fql"
        }
      },

Of course there is some additional handling imposed, but you get the picture.

Improve scopes

Let's take a quick look at .fsl wherein extension is not applied to textmate scope of variable.other.property

Screenshot 2023-12-08 at 20 45 22

This can be problematic in cases where a developer is leveraging editor.tokenColorCustomizations and wants to target this scope specifically. Such logic can easily be patched by simply ensuring token names always apply .fsl or .fql suffixes. Moving on, the grammars can generally be improved with better captures and matching applied, I'd be happy to provide a list or overhaul.

IntelliSense / FQL Language Server

I've touched on this previously in the linked issues of now deprecates/legacy tooling. The fql-analyzer from what I can tell is responsible for completions and other LSP capabilities like validations (which is great) however the implementation is void of descriptions and hover features. This makes things very difficult given the size of the FQL spec and would be of great value if exposed.

The Fauna Shell via the Dashboard is leveraging CodeMirror, which supports the protocol and by supporting descriptions and additional capabilities (i.e: hovers) both the editor (this extension) and the shell (dashboard) toolings benefit. The documentation already provides good, short and informative texts, and given LSP can digest Markdown descriptions, (which I'd assume the official docs are composed with) can we see about introducing them based on the documentation that already available?

Lastly, the fql`` literal can also benefit from the existing fql-analyzer capabilities. This will require some mild parsing to obtain occurrences within TextDocuments of the editor. Typically lexing is done on the language server, but this can also be done on this client, in any sense are there plans to open source this (https://static-assets.fauna.com/fql-analyzer/index.js) so support can be made available in literals.

Thanks

@macmv
Copy link
Collaborator

macmv commented Dec 8, 2023

Sorry about not replying to your past issues! Just as a quick reply to those two issues:

  • We've been prioritizing our efforts towards the fauna-vscode package, as the vscode package is only for v4. We plan to migrate all functionality from that package to this one.
  • We've been focusing on FQL v10, so v4 improvements haven't been a priority. FQL v10 functions will have support for doc-comments through schema, which isn't implemented in fql-analyzer yet, but we have plans for it.

These grammars were written to work for fql files. We haven't done a ton of research into support fql snippets in javascript files, but from what you've posted here it doesn't look very difficult. This is definitely something we'd like to support.

If you have any changes to improve the captures or provided scopes, we'd happily accept a PR or list of issues. I wrote these grammars a few months ago, and they definitely haven't gotten rigorous testing or anything like that.

As for fql-analyzer, yes, this is what provides completions and inline errors. This is closed source because it depends directly on the FQL v10 parser. The dashboard embeds an instance of fql-analyzer internally, so all the completions and errors come from the same place. We haven't prioritized support for hover or inline documentation in fql-analyzer, but we realize these features would be nice to have.

Our docs are written with asciidoc, so we can't just directly include them. Internally, we need to convert them to markdown, and line them up with the structure of the standard library, which we haven't prioritized.

Supporting fql blocks in fql-analyzer is also a feature we looked into in depth, but supporting templated values ends up being very difficult. fql-analyzer runs a typechecker across the entire query, and template values have a static type, but we don't know what that static type is unless we hook into a typescript language server of some kind. In practice, it would end up being difficult to parse reliably, and it wouldn't be particularly accurate.

@panoply
Copy link
Author

panoply commented Dec 8, 2023

Hey!

Thanks a ton for the speedy reply. I understand you are only a small team, but it did bother me being left on read previously, though it makes total sense now. Anyway, I really appreciate the communication. I'll take this portions.

If you have any changes to improve the captures or provided scopes, we'd happily accept a PR or list of issues. I wrote these grammars a few months ago, and they definitely haven't gotten rigorous testing or anything like that.

I'll have a look over the weekend and PR on grammars, it's easier, faster and quicker to extend upon the existing logic. I'll also bringing support to literals.

This is closed source because it depends directly on the FQL v10 parser.

Understood.

We haven't prioritized support for hover or inline documentation in fql-analyzer, but we realize these features would be nice to have. Our docs are written with asciidoc, so we can't just directly include them. Internally, we need to convert them to markdown, and line them up with the structure of the standard library, which we haven't prioritized.

LSP can also digest basic markup here. I did give thought to manually composing some basic JSON structures directly from the docs in v4 tooling when I considered building out an alternative. Extracting baseline descriptives and from here passing to completion item in a raw format, however manually scaping would inevitably be problematics as any changes would need to align. Nonetheless, I'd be willing to give it a go but would need more context.

Supporting fql blocks in fql-analyzer is also a feature we looked into in depth, but supporting templated values ends up being very difficult. fql-analyzer runs a typechecker across the entire query, and template values have a static type, but we don't know what that static type is unless we hook into a typescript language server of some kind. In practice, it would end up being difficult to parse reliably, and it wouldn't be particularly accurate.

Ah! shit, I totally neglected the static type aspect, it would indeed be challenging to get right. If I understand correctly though, the fql-analyzer requires the entirety of the query, it cannot perform incremental analysis? fql-blocks can live without supporting some features but can also appropriate others, but I'm merely speculating as I have no context of the parser. It's a very interesting and difficult problem that I'm very much interested in.

@cleve-fauna
Copy link
Contributor

Hey we've done some initial work on fql-blocks in javascript / typescript here:

#37

@cleve-fauna
Copy link
Contributor

Hey @panoply the updated VS code extension with this syntax highlighting is here: https://marketplace.visualstudio.com/items?itemName=fauna.fauna-vscode

@panoply
Copy link
Author

panoply commented Oct 21, 2024

We no longer use Fauna (v10 was just too problematic) but while migrating away from the service I noticed the below.

  1. Punctuations are not tokenized
  2. Keywords are not tokenized.
  3. Integers, floats, and various types are not tokenized.
  4. Zero use of inferred meta
Screenshot 2024-10-21 at 23 36 20

Closing this.

@panoply panoply closed this as completed Oct 21, 2024
@panoply
Copy link
Author

panoply commented Oct 29, 2024

We are still using Fauna in some projects so until we completely migrate away, I'll help:

See #43

@panoply panoply reopened this Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants