Export lintState StateField

thetrevorharmon commented

2022-07-22 21:16:53 +02:00

(Migrated from github.com)

This PR exports the lintState StateField, which enables users to lookup the state of that field view the view.state.field(lintState) mechanism.

While building out the ability to provide information via tooltips, I ran into an issue with styling the tooltips that come out of the lint function. This discussion thread describes a change that enabled combining all tooltips into a single container, which ensures they don't overlap. However, there is not currently a mechanism for directly intercepting and controlling the content of a lint hover tooltip at the same time as a normal hover tooltip.

My initial approach was to create a new state field and watch the setDiagnosticsEffect, and then set that state field to the value of the new diagnostics. However, I found that there was a race condition between the time that a hover tooltip started and the time that the state field was updated–I was seeing the hover tooltip called before the state field update.

Here's what that code looked like

export const diagnosticStateField = StateField.define<
  ReadonlyArray<Diagnostic>
>({
  create: (_: EditorState) => [],

  update(value, transaction) {
    if (transaction.docChanged === true) {
      let newDiagnostics: ReadonlyArray<Diagnostic> = [];

      transaction.effects.map((effect) => {
        if (effect.is(setDiagnosticsEffect)) {
          newDiagnostics = effect.value;
        }
      });

      return newDiagnostics;
    }

    return value;
  },
});

After doing some additional research, I found that I could do all of the work I needed to do if I had access to the lintState field, via the .field accessor on state. Rather than duplicating this into its own locally-defined state field, it would be better to simply access the existing one.

Because the properties on lintState are read-only, it seemed safe to expose this outside of the library.

This PR exports the `lintState` StateField, which enables users to lookup the state of that field view the `view.state.field(lintState)` mechanism. While building out the ability to provide information via tooltips, I ran into an issue with styling the tooltips that come out of the lint function. [This discussion thread](https://discuss.codemirror.net/t/cm6-merging-lint-and-hover-tooltips-over-the-same-range/3010) describes a change that enabled combining all tooltips into a single container, which ensures they don't overlap. However, there is not currently a mechanism for directly intercepting and controlling the content of a lint hover tooltip at the same time as a normal hover tooltip. My initial approach was to create a new state field and watch the `setDiagnosticsEffect`, and then set that state field to the value of the new diagnostics. However, I found that there was a race condition between the time that a hover tooltip started and the time that the state field was updated–I was seeing the hover tooltip called before the state field update. <details> <summary>Here's what that code looked like</summary> ```tsx export const diagnosticStateField = StateField.define< ReadonlyArray<Diagnostic> >({ create: (_: EditorState) => [], update(value, transaction) { if (transaction.docChanged === true) { let newDiagnostics: ReadonlyArray<Diagnostic> = []; transaction.effects.map((effect) => { if (effect.is(setDiagnosticsEffect)) { newDiagnostics = effect.value; } }); return newDiagnostics; } return value; }, }); ``` </details> After doing some additional research, I found that I could do all of the work I needed to do if I had access to the `lintState` field, via the `.field` accessor on `state`. Rather than duplicating this into its own locally-defined state field, it would be better to simply access the existing one. Because the properties on `lintState` are read-only, it seemed safe to expose this outside of the library.

thetrevorharmon commented

2022-07-22 21:38:13 +02:00

(Migrated from github.com)

I added documentation, hopefully it is accurate to what the PR is changing.

marijnh commented

2022-07-25 12:22:38 +02:00

(Migrated from github.com)

Could you go into a bit more details about your use case? Exporting this would also require exporting LintState, as well as the fact that the code stashes the annotations in the decoration marker, so I'd prefer not to do it like this.

Could you go into a bit more details about your use case? Exporting this would also require exporting `LintState`, as well as the fact that the code stashes the annotations in the decoration marker, so I'd prefer not to do it like this.

thetrevorharmon commented

2022-07-25 22:26:09 +02:00

(Migrated from github.com)

Could you go into a bit more details about your use case?

Right now we have two types of tooltips–informational and error. Informational describes what the token is/does (e.g. if it's a keyword) and error shows if the token is malformed in some way. By default, codemirror displays two separate tooltips (rendered in the same container) for each of these things. My aim is to get diagnostic information out of the editor state in the place where we call hoverTooltip, and then construct my own custom tooltip that does some particular manipulation of the "information" portion and "error" portion.

Exporting this would also require exporting LintState, as well as the fact that the code stashes the annotations in the decoration marker, so I'd prefer not to do it like this.

That's understandable. I explored a couple of options and was wondering what you thought about them

Option 1: `diagnosticList`

The idea behind this option is to export a function similar to diagnosticCount that would export the diagnostics.

export function diagnosticList(state: EditorState) {
  const lint = state.field(lintState, false);
  const diagnostics: Diagnostic[] = [];

  if (lint != null) {
    lint.diagnostics.between(0, state.doc.length, (_to, _from, value) => {
      diagnostics.push(value.spec.diagnostic);
    });
  }

  return diagnostics;
}

Option 2: `diagnosticState`

In this option, you would internally utilize a function like diagnosticList but you would be keeping track of the diagnostics in state:

export const diagnosticState = StateField.define<Diagnostic[]>({
  create() {
    return [];
  },
  update(value, tr) {
    for (const effect of tr.effects) {
      if (effect.is(setDiagnosticsEffect)) {
        value = diagnosticList(tr.state);
      }
    }

    return value;
  },
});

In this example, you would use this like view.state.field(diagnosticState). It probably is less convenient than a diagnosticList function, but it might lead to better performance since it only changes the diagnostics when the effect is triggered.

In both of these examples, you don't export the internals of lintState and there are no additional types to export (since Diagnostic is already exported).

What do you think?

> Could you go into a bit more details about your use case? Right now we have two types of tooltips–informational and error. Informational describes what the token is/does (e.g. if it's a keyword) and error shows if the token is malformed in some way. By default, codemirror displays two separate tooltips (rendered in the same container) for each of these things. My aim is to get diagnostic information out of the editor state in the place where we call `hoverTooltip`, and then construct my own custom tooltip that does some particular manipulation of the "information" portion and "error" portion. > Exporting this would also require exporting LintState, as well as the fact that the code stashes the annotations in the decoration marker, so I'd prefer not to do it like this. That's understandable. I explored a couple of options and was wondering what you thought about them ### Option 1: `diagnosticList` The idea behind this option is to export a function similar to `diagnosticCount` that would export the diagnostics. ```tsx export function diagnosticList(state: EditorState) { const lint = state.field(lintState, false); const diagnostics: Diagnostic[] = []; if (lint != null) { lint.diagnostics.between(0, state.doc.length, (_to, _from, value) => { diagnostics.push(value.spec.diagnostic); }); } return diagnostics; } ``` ### Option 2: `diagnosticState` In this option, you would internally utilize a function like `diagnosticList` but you would be keeping track of the diagnostics in state: ```tsx export const diagnosticState = StateField.define<Diagnostic[]>({ create() { return []; }, update(value, tr) { for (const effect of tr.effects) { if (effect.is(setDiagnosticsEffect)) { value = diagnosticList(tr.state); } } return value; }, }); ``` In this example, you would use this like `view.state.field(diagnosticState)`. It probably is less convenient than a `diagnosticList` function, but it might lead to better performance since it only changes the diagnostics when the effect is triggered. In both of these examples, you don't export the internals of `lintState` and there are no additional types to export (since `Diagnostic` is already exported). What do you think?

thetrevorharmon commented

2022-07-26 16:51:09 +02:00

(Migrated from github.com)

I worked on this more and discovered I can do this with a custom state field after all. I modified the state field so it looks like this:

export const diagnosticStateField = StateField.define<
  ReadonlyArray<Diagnostic>
>({
  create: (_: EditorState) => [],

  update(value, transaction) {
    let diagnostics = value;

    transaction.effects.map((effect) => {
      if (effect.is(setDiagnosticsEffect)) {
        diagnostics = effect.value;
      }
    });

    return diagnostics;
  },
});

I was previously wrapping the logic in a check against transaction.docChanged, but after looking through the source of this package, I observed that wasn't what is being done for updating the diagnostics. In an ideal world, it would be nice to be able to get at the diagnostic state from the package instead of creating an additional state field, but this is working for now.

If you still have interest in exposing a utility like this out of the package, I'm happy to modify this PR with whatever approach you feel is best. However, if you aren't interested in pursuing that idea, feel free to close the PR.

I worked on this more and discovered I can do this with a custom state field after all. I modified the state field so it looks like this: ```tsx export const diagnosticStateField = StateField.define< ReadonlyArray<Diagnostic> >({ create: (_: EditorState) => [], update(value, transaction) { let diagnostics = value; transaction.effects.map((effect) => { if (effect.is(setDiagnosticsEffect)) { diagnostics = effect.value; } }); return diagnostics; }, }); ``` I was previously wrapping the logic in a check against `transaction.docChanged`, but after looking through the source of this package, I observed that wasn't what is being done for updating the diagnostics. In an ideal world, it would be nice to be able to get at the diagnostic state from the package instead of creating an additional state field, but this is working for now. If you still have interest in exposing a utility like this out of the package, I'm happy to modify this PR with whatever approach you feel is best. However, if you aren't interested in pursuing that idea, feel free to close the PR.

marijnh commented

2022-07-27 10:27:04 +02:00

(Migrated from github.com)

Let's close until other use cases surface, then.

Pull request closed

Please reopen this pull request to perform a merge.

Rows
Columns

Export lintState StateField #7

Option 1: diagnosticList

Option 2: diagnosticState

Pull request closed

Option 1: `diagnosticList`

Option 2: `diagnosticState`