Skip to content
This repository was archived by the owner on Apr 4, 2023. It is now read-only.

Conversation

sangxxh
Copy link

@sangxxh sangxxh commented Mar 14, 2021

Close #119

Implementation

  • Update vscode-css-languageservice to v5.1.0.
  • Also update vscode-languageserver-types to v3.16.0 due to compatibility with vscode-css-languageservice

@sangxxh
Copy link
Author

sangxxh commented Mar 14, 2021

Both before and after updating packages, running e2e tests fail for me:

  63 passing (2m)
  1 failing

  1) OutliningSpans
       should return basic css outlining spans:

      AssertionError: expected 3 to equal 2
      + expected - actual

      -3
      +2

      at Context.<anonymous> (e2e\tests\outliningSpans.js:22:16)
      at processTicksAndRejections (internal/process/task_queues.js:93:5)

Is this expected on main branch?

@sangxxh sangxxh changed the title chore: update vscode-css-languageservice build: update vscode-css-languageservice Mar 27, 2021
@jasonwilliams
Copy link
Contributor

jasonwilliams commented Apr 26, 2021

Both before and after updating packages, running e2e tests fail for me:

@hantatsang i had the same issue which is a shame, are you able to update the dependencies again and I can take a look at the test

@sangxxh
Copy link
Author

sangxxh commented Apr 26, 2021

@jasonwilliams I just to upgraded to vscode-css-languageservice@5.1.1 (latest). Still, the same test fails 🤔

@sangxxh
Copy link
Author

sangxxh commented Apr 26, 2021

All my tests also failed at first after this upgrade. But then I cd e2e && npm i, run test again and only the 1 test above failed

@jasonwilliams
Copy link
Contributor

jasonwilliams commented Apr 26, 2021

All my tests also failed at first after this upgrade. But then I cd e2e && npm i, run test again and only the 1 test above failed

Yep i just noticed that too. I have the same single test failing now, thanks.

@jasonwilliams
Copy link
Contributor

jasonwilliams commented Apr 26, 2021

Looks like it used to return 2 spans at one point, now it returns 3. Here's what spans looks like:

[
  {
    textSpan: {
      start: {
        line: 1,
        offset: 14,
      },
      end: {
        line: 8,
        offset: 2,
      },
    },
    hintSpan: {
      start: {
        line: 1,
        offset: 14,
      },
      end: {
        line: 8,
        offset: 2,
      },
    },
    bannerText: "...",
    autoCollapse: false,
    kind: "code",
  },
  {
    textSpan: {
      start: {
        line: 2,
        offset: 1,
      },
      end: {
        line: 3,
        offset: 1,
      },
    },
    hintSpan: {
      start: {
        line: 2,
        offset: 1,
      },
      end: {
        line: 3,
        offset: 1,
      },
    },
    bannerText: "",
    autoCollapse: false,
    kind: "code",
  },
  {
    textSpan: {
      start: {
        line: 5,
        offset: 1,
      },
      end: {
        line: 6,
        offset: 1,
      },
    },
    hintSpan: {
      start: {
        line: 5,
        offset: 1,
      },
      end: {
        line: 6,
        offset: 1,
      },
    },
    bannerText: "",
    autoCollapse: false,
    kind: "code",
  },
]

@jasonwilliams
Copy link
Contributor

jasonwilliams commented Apr 26, 2021

@hantatsang i think the new output (above) is correct. It seems like it creates an extra span for the whole thing. So the test can be updated to accept that. Im guessing when this was originally written only the 2 inner blocks generated spans, but now it generates a span for the const q definition.

pings @mjbvz for confirmation

@jasonwilliams
Copy link
Contributor

Also @hantatsang does npm run compile work for you? I don't have your change checked out but i remember i was having issues before with that. Hopefully the types are now updated in tandem with css-language service

@jasonwilliams
Copy link
Contributor

jasonwilliams commented Apr 26, 2021

@hantatsang could you change outliningSpans.js to this

        assert.strictEqual(spans.length, 3);
        // The first span represents the root
        const [_, span2, span3] = spans;
        assertPosition(span2.textSpan.start, 2, 1);
        assertPosition(span2.textSpan.end, 3, 1);

        assertPosition(span3.textSpan.start, 5, 1);
        assertPosition(span3.textSpan.end, 6, 1);

Then I think we're good

@sangxxh
Copy link
Author

sangxxh commented Apr 27, 2021

Thanks, @jasonwilliams . Updated. Hope this will get reviewed & merged.

@jasonwilliams
Copy link
Contributor

@mjbvz are you able to look at this also?

@mjbvz mjbvz merged commit c9d106e into microsoft:main Apr 28, 2021
@mjbvz
Copy link
Contributor

mjbvz commented Apr 28, 2021

Thanks!

@sangxxh sangxxh deleted the issue/119-update-vscode-css-languageservice branch April 29, 2021 03:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update vscode-css-language service to 4.3.0
3 participants