Add notification on corelib mismatch

[piotmag769]

Closes #6174

---

This change is 

[piotmag769]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Arcticae, @Draggu, @mkaput, and @orizi)

[orizi]

Reviewed 3 of 3 files at r1.
Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @Arcticae, @Draggu, @mkaput, and @piotmag769)

[Draggu]

Reviewable status: 2 of 3 files reviewed, 4 unresolved discussions (waiting on @Arcticae, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/lib.rs line 605 at r2 (raw file):

                let corelib_validation = validate_corelib(db);
                if let Err(result) = corelib_validation {
                    let notifier = Notifier::new(&self.client);

---

crates/cairo-lang-language-server/src/lib.rs line 608 at r2 (raw file):

                    spawn_blocking(move || {
                        notifier.send_notification::<CorelibVersionMismatch>(result.to_string());
                    });

Notifier is wrapper that makes it synchronous, if you want make it async just use client directly

Suggestion:

                    self.client.send_notification::<CorelibVersionMismatch>(result.to_string()).await;

---

vscode-cairo/src/cairols.ts line 86 at r2 (raw file):

  });

  client.onNotification(new NotificationType<string>("corelib/version-mismatch"), (params) => {

Make it clear that payload of this notification is error message from server

Suggestion:

errMessage

---

vscode-cairo/src/cairols.ts line 89 at r2 (raw file):

    const errorMessage =
      "Corelib version mismatch. If you are using Scarb try reopening the project to fix this error. Resort to `scarb cache clean` if it doesn't help. ERROR: " +
      params;

Suggestion:

errMessage

[orizi]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Arcticae, @mkaput, and @piotmag769)

[mkaput]

Reviewed 1 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Arcticae, @Draggu, and @piotmag769)

---

crates/cairo-lang-language-server/src/lib.rs line 608 at r2 (raw file):

Previously, Draggu (Piotr Figiela) wrote…

Notifier is wrapper that makes it synchronous, if you want make it async just use client directly

Not exactly. I want Notifier to be the object for sending notifications long term (which is separate from client to reduce scope). It's sync because we are aiming to refactor our entire codebase to sync world (as part of lsp-server migration).

But your code suggestion is totally valid here!

[mkaput]

Reviewed 1 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @Arcticae, @Draggu, and @piotmag769)

---

crates/cairo-lang-language-server/src/lib.rs line 604 at r2 (raw file):

                let corelib_validation = validate_corelib(db);
                if let Err(result) = corelib_validation {

nit

Suggestion:

                if let Err(result) = validate_corelib(db) {

---

crates/cairo-lang-language-server/src/lib.rs line 662 at r2 (raw file):

#[derive(Debug)]
struct CorelibVersionMismatch {}

I'm thinking if we shouldn't move all our LSP extensions to crates/cairo-lang-language-server/src/lsp/ext.rs, so that they all will be discoverable.

Forget about Scarb notifications - they are to be redone.

---

crates/cairo-lang-language-server/src/lib.rs line 666 at r2 (raw file):

impl Notification for CorelibVersionMismatch {
    type Params = String;
    const METHOD: &'static str = "corelib/version-mismatch";

I noticed LSP extensions tend to use the part before / for namespacing, so let's establish that all our custom stuff is cairo/. (RA has rust-analyzer/)

Forget about Scarb notifications - they are to be redone.

Suggestion:

cairo/corelib-version-mismatch

---

vscode-cairo/src/cairols.ts line 88 at r2 (raw file):

  client.onNotification(new NotificationType<string>("corelib/version-mismatch"), (params) => {
    const errorMessage =
      "Corelib version mismatch. If you are using Scarb try reopening the project to fix this error. Resort to `scarb cache clean` if it doesn't help. ERROR: " +

Avoid term corelib in user-facing communication. It's a "core crate".

---

vscode-cairo/src/cairols.ts line 88 at r2 (raw file):

  client.onNotification(new NotificationType<string>("corelib/version-mismatch"), (params) => {
    const errorMessage =
      "Corelib version mismatch. If you are using Scarb try reopening the project to fix this error. Resort to `scarb cache clean` if it doesn't help. ERROR: " +

• What does this message look like in real life?
• Can you post a screenshot?
• From bare looking at this, this looks to be ugly. Can it be markdown?
• Can you split this into paragraphs: User digestible gist of the problem, Technical full reason, Troubleshooting steps?

[orizi]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @Arcticae, @Draggu, and @piotmag769)

[mkaput]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Arcticae, @Draggu, and @piotmag769)

[piotmag769]

Reviewed 1 of 1 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Arcticae, @Draggu, and @mkaput)

---

crates/cairo-lang-language-server/src/lib.rs line 604 at r2 (raw file):

Previously, mkaput (Marek Kaput) wrote…

nit

Done.

---

crates/cairo-lang-language-server/src/lib.rs line 605 at r2 (raw file):

                let corelib_validation = validate_corelib(db);
                if let Err(result) = corelib_validation {
                    let notifier = Notifier::new(&self.client);

Done.

---

crates/cairo-lang-language-server/src/lib.rs line 608 at r2 (raw file):

Previously, mkaput (Marek Kaput) wrote…

Not exactly. I want Notifier to be the object for sending notifications long term (which is separate from client to reduce scope). It's sync because we are aiming to refactor our entire codebase to sync world (as part of lsp-server migration).

But your code suggestion is totally valid here!

Done.

---

crates/cairo-lang-language-server/src/lib.rs line 662 at r2 (raw file):

Previously, mkaput (Marek Kaput) wrote…

I'm thinking if we shouldn't move all our LSP extensions to crates/cairo-lang-language-server/src/lsp/ext.rs, so that they all will be discoverable.

Forget about Scarb notifications - they are to be redone.

Done.

---

crates/cairo-lang-language-server/src/lib.rs line 666 at r2 (raw file):

Previously, mkaput (Marek Kaput) wrote…

I noticed LSP extensions tend to use the part before / for namespacing, so let's establish that all our custom stuff is cairo/. (RA has rust-analyzer/)

Forget about Scarb notifications - they are to be redone.

Done.

---

vscode-cairo/src/cairols.ts line 86 at r2 (raw file):

Previously, Draggu (Piotr Figiela) wrote…

Make it clear that payload of this notification is error message from server

Done.

---

vscode-cairo/src/cairols.ts line 88 at r2 (raw file):

Previously, mkaput (Marek Kaput) wrote…

Avoid term corelib in user-facing communication. It's a "core crate".

Done.

---

vscode-cairo/src/cairols.ts line 88 at r2 (raw file):

Can it be a markdown?
Can you split this into paragraphs

Not really

microsoft/vscode#48900 (comment)
microsoft/vscode#50512 (comment)

---

vscode-cairo/src/cairols.ts line 89 at r2 (raw file):

    const errorMessage =
      "Corelib version mismatch. If you are using Scarb try reopening the project to fix this error. Resort to `scarb cache clean` if it doesn't help. ERROR: " +
      params;

Done.

[mkaput]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @Arcticae, @Draggu, and @piotmag769)

---

vscode-cairo/src/cairols.ts line 87 at r3 (raw file):

  client.onNotification(
    new NotificationType<string>("cairo/corelib-version-mismatch"),

is there a reason why are you using NotificationType here? just curious

Suggestion:

"cairo/corelib-version-mismatch"

[mkaput]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @Arcticae, @Draggu, and @piotmag769)

---

vscode-cairo/src/cairols.ts line 88 at r2 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

Can it be a markdown?
Can you split this into paragraphs

Not really

microsoft/vscode#48900 (comment)
microsoft/vscode#50512 (comment)

I see you can register actions/buttons. Perhaps we should add buttons for reloading the window and/or running scarb cache clean. Wdyt?

[mkaput]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Arcticae, @Draggu, and @piotmag769)

---

vscode-cairo/src/cairols.ts line 87 at r3 (raw file):

Previously, mkaput (Marek Kaput) wrote…

is there a reason why are you using NotificationType here? just curious

Oh I see, for typing params. Cool

[piotmag769]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Arcticae, @Draggu, and @mkaput)

---

vscode-cairo/src/cairols.ts line 88 at r2 (raw file):

Previously, mkaput (Marek Kaput) wrote…

I see you can register actions/buttons. Perhaps we should add buttons for reloading the window and/or running scarb cache clean. Wdyt?

Good idea, will do

[piotmag769]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Arcticae, @Draggu, and @mkaput)

---

vscode-cairo/src/cairols.ts line 88 at r2 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

Good idea, will do

Done.

[piotmag769]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Arcticae, @Draggu, and @mkaput)

[mkaput]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Arcticae, @Draggu, and @piotmag769)

---

vscode-cairo/src/cairols.ts line 94 at r4 (raw file):

        errMessage;

      ctx.log.error(errorMessage);

IMHO now the extra stuff you put in the error message does not bring anything, so it can be removed

Suggestion:

    async (errorMessage) => {
      ctx.log.error(errorMessage);

[mkaput]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Arcticae and @Draggu)

[piotmag769]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Arcticae and @Draggu)

---

vscode-cairo/src/cairols.ts line 94 at r4 (raw file):

Previously, mkaput (Marek Kaput) wrote…

IMHO now the extra stuff you put in the error message does not bring anything, so it can be removed

Done.

[mkaput]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Arcticae and @Draggu)

[Draggu]

Reviewable status: 3 of 4 files reviewed, all discussions resolved (waiting on @Arcticae and @mkaput)

[piotmag769]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Arcticae)