opral / inlang-sherlock

Issue tracker for Sherlock
https://marketplace.visualstudio.com/items?itemName=inlang.vs-code-extension
2 stars 0 forks source link

How we get input declarations in the AST as devs #74

Open NilsJacobsen opened 1 month ago

NilsJacobsen commented 1 month ago

When developers add variables to a m-function they might think that this input declaration is also available in the ast (aka in the project for all other apps). But that is not the case.

Example

Developer knows that there is a count variable in the ref language so he adds the count variable right to the m function. At this point when I open fink, there is no count available as an input declariation in the ast. Im basically not able to use count as referencePattern or a selector unless I added the input again manually.

Proposal 1: Add a lint rule that is resolvable.

When I argument is passed into a m-function without a input declaration definition in the ast it get's linted. You can auto resolve the lint by adding it to the ast.

Proposal 2: Add a vs-code shortcut

Similar to the extract mechanism we could also add the input declaration in the ast and in the code with one step.

I fell like the lint rule would be the smoother solution, but I don't know how long we need to wait for it. This feature is needed at the time of message bundle component rollout, because this is such a basic use case.

felixhaeberle commented 1 month ago

You mean how to extract this in Sherlock?

I have two donuts. 🍩 = m.donut_count({count: 2})

samuelstroschein commented 1 month ago

Yep. Sherlock must have a way to extract declarations too/have a keyboard navigation thingy for devs to do it fast

felixhaeberle commented 1 month ago

I disagree on that.

A static string is a normal string without any declaration. This string can later be made to something more through editing and message function can then also consume variables through props, but specify that at "message create/extract" goes against our "first simple message" approach where we add complexity gradually through the message component and not upfront.

samuelstroschein commented 1 month ago

@felix.haeberle code will have this

<p>You have {numberPhotos} photos</p>

you need to extract {numberPhotos} in sherlock as a declaration

felixhaeberle commented 1 month ago

You can do this in the message component? Also its not that easy. You now also have to specify whether it is photo/photos in this string for example. You have to jump in the message component either way for that.

felixhaeberle commented 1 month ago

At first, I would default to "edit in message component", and only after people request more VS Code built-in functionality: build it.

samuelstroschein commented 1 month ago

that means that @nils.jacobsen needs to build a "extract declarations" feature into the message component

samuelstroschein commented 1 month ago

i leave it up to nils on what to do

NilsJacobsen commented 1 month ago

Scenarios:

felixhaeberle commented 1 month ago

that means that @nils.jacobsen needs to build a "extract declarations" feature into the message component

Not extract, why extract? Just create a declaration and use it in the message. Please don't overcomplicate this UX with users who have complete different setup where it gets really hard to do this (extract) right across librarier and potentially across languages.

  • Declaration is written in extractable message, get's lost when extracted.

Expected, how should Sherlock know?

  • Dev wants to add a new declaration to m function ⚡️ this is not possible (it doesn't get in the ast, their expectation will be: it's passed in, why doesn't it work?)

Same as above

  • Auto extraction would cause problems if it is not capable of doing this

No problem as it is just not supported and you have to go through it afterwards.

I still don't get why we should overload the complexity of a simple text extract / message create when this is expected to be gradually added and not upfront.

@nils.jacobsen Potentially a LOOM could help to understand your requirement more, or a small session in Around.

samuelstroschein commented 1 month ago

@nils.jacobsen correct me if my loom is wrong. we want 3 things:

  1. sherlock ideally extracts a message with correct declarations
  2. we want a lint rule that detects "no declaration but variable reference used"
  3. the message editor component ideally has a "extract as declaration" feature

https://www.loom.com/share/a73d0144b4324fdaa559a15dd568a686?sid=6a2f2868-77e3-447b-b3db-861183b3dbd1

NilsJacobsen commented 1 month ago

Small addition on the loom:

https://www.loom.com/share/78f08810532b4b0996f2669a27d4897c?sid=1dae3b31-478c-4ed1-957b-0dcd906c2010

felixhaeberle commented 1 month ago

@samuel.stroschein @nils.jacobsen Okay folks we are mixing quite a few things here & have reached a scope way above Sherlock. Let me summarize this discussion, we need:

Parser

  1. A variable reference parser which parsers strings in multiple formats & potentially multiple languages to be used by Sherlock when extracting strings and a potential lint rule to detect variable usages in existing keys.

Lint Rule

  1. A lint rule which uses a variable reference parser to display lints based on content of a string

App Specifc Functionality (Sherlock)

  1. A Sherlock "Quick Action" in Code for Declaration creation per message to not have to jump back & forth between the message component and the code.

I would not advise for [3] upfront because why should somebody want go "backwards" the compilation step of the message function. This feels like the wrong DX/UX. Also, thinking about other libraries we need to support further: they also always start on the content/key side. See i18next.

Bildschirmfoto 2024-06-07 um 11.34.42.png


So the mental model is always:

  1. Define declaration in content (message component in our case)
  2. Use declaration in code
NilsJacobsen commented 1 month ago

Let's have a short call after lunch :)

felixhaeberle commented 1 month ago

yep, time? 13:00?

NilsJacobsen commented 1 month ago

Summary from the call:

  1. We need to make sure that the communication between dev and ast infromation is working via type lints in m function (maybe show a link to edit or quickfix problems)
    @loris.sigrist how is paraglide doing type checks, lint.
  2. We can work on the extraction of variables
felixhaeberle commented 1 month ago

@NilsJacobsen Can you create follow-up issues that we can close this discussion here?

NilsJacobsen commented 4 weeks ago

Gonna talk to Loris about it and then add it to the right team.

LorisSigrist commented 3 weeks ago

I don't think encouraging devs to add extra variables to messages is a good idea as we are not able to consume all possible JS values. What if someone passes a user object instead of just the name? How do we detect the type of a value without reimplementing TS? This makes any "extraction" based approach (like a lint rule) undesirable.

Editing messages & making extra inputs available should be done in the same place that new messages are created. For devs that's Sherlock or files.

NilsJacobsen commented 3 weeks ago

The question is, can we already lint if a user didn't add the required inputs?

LorisSigrist commented 3 weeks ago

The extra values would cause a type-error in the code. The dev already sees they're adding extra values & will just updated the message AST with the inputs.

Non-Devs wouldn't benefit from this lint rule since code with type-errors won't be committed.

I don't see a benefit to a lint rule.

NilsJacobsen commented 3 weeks ago

The extra values would cause a type-error in the code.

This is already working right?

I don't see a benefit to a lint rule.

I don't think somebody is proposing that 😀

NilsJacobsen commented 3 weeks ago

@felix.haeberle would it be possible to add a link/quickfix to the type error modal via Sherlock? How can these be adjusted?

felixhaeberle commented 3 weeks ago

@NilsJacobsen please follow the discussion we have in https://linear.app/opral/issue/SHERL-82/support-import-m-from-on-vscode-extension#comment-4082ba93 to learn about the CompletionItem & CompletionProvider.

See also: https://code.visualstudio.com/api/references/vscode-api#CompletionItemProvider