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

15$: Website: Show error messages #258

Closed
strager opened this issue May 1, 2021 · 11 comments · Fixed by #364
Closed

15$: Website: Show error messages #258

strager opened this issue May 1, 2021 · 11 comments · Fixed by #364
Assignees
Labels
for hire Get paid for working on this task: https://quick-lint-js.com/hiring.html

Comments

@strager
Copy link
Collaborator

strager commented May 1, 2021

This task involves writing browser JS.

In the web demo (https://quick-lint-js.com/demo/), we highlight errors, but we give no indication what the error is. Include a message somewhere in the UI so people can understand the errors reported by quick-lint-js.

@strager strager added the for hire Get paid for working on this task: https://quick-lint-js.com/hiring.html label May 1, 2021
@matttheus
Copy link
Contributor

hello strager

I claim this for-hire task. I expect payment after I complete this task. I will email the quick-lint-js team if I am assigned this task.

@matttheus
Copy link
Contributor

matttheus commented May 7, 2021

I was playing around with the demo and this show up when I wrote function test(){}. There is some bug here or I miss configure something?

image

Edit 1: I am build it locally

Edit 2: I guess this error is related to this TODO at website/demo/index.html file, am I right?

loadQuickLintJS()
  .then((quickLintJS) => {
    function lintAndUpdate() {
      synchronizeContent();

      // TODO(strager): On crash, show the error to the user.
      let input = codeInputElement.value;
      let marks = quickLintJS.parseAndLint(input);
      markEditorText(shadowCodeInputElement, window, marks);
    }
    codeInputElement.addEventListener("input", (event) => {
      lintAndUpdate();
    });
    lintAndUpdate();
  })
  .catch((error) => {
    // TODO(strager): Show this error to the user.
    console.error(error);
  });

@matttheus
Copy link
Contributor

Where you suggest I look to find where the errors come from? I looked up at website/demo/editor.mjs and website/demo/index.html but I could not figure out where this error message come from.

I suppose the error should come from this line of code from website/demo/index.html.

let marks = quickLintJS.parseAndLint(input);

Can you help figure out this?

@strager
Copy link
Collaborator Author

strager commented May 7, 2021

I was playing around with the demo and this show up when I wrote function test(){}. There is some bug here or I miss configure something?

image

Edit 1: I am build it locally

That's quick-lint-js crashing. #109 tracks fixing this.

Edit 2: I guess this error is related to this TODO at website/demo/index.html file, am I right?

Yes. But that's not related to this task (#258).

@strager
Copy link
Collaborator Author

strager commented May 7, 2021

Where you suggest I look to find where the errors come from? I looked up at website/demo/editor.mjs and website/demo/index.html but I could not figure out where this error message come from.

I think the unreachable executed exception comes from a call to QLJS_UNREACHABLE. I assume the error message itself comes from emscripten. (But this exception is not related to this task.)

@strager
Copy link
Collaborator Author

strager commented May 7, 2021

I suppose the error should come from this line of code from website/demo/index.html.

let marks = quickLintJS.parseAndLint(input);

This task is talking about marks. marks includes error locations, but I think it doesn't include error messages. This task involves making quickLintJS.parseAndLint return the messages, and having the messages appear in the UI.

The (non-LSP) VS Code plugin (in vscode/plugin/) shows messages. Perhaps we can copy what the VS Code plugin does to extract messages from the C++ code.

@strager
Copy link
Collaborator Author

strager commented May 13, 2021

I plan on adding some features to the vscode.h interface. I might switch the web demo over to the vscode.h interface as a result. If I do that, I think this task should be easier.

@matttheus
Copy link
Contributor

My plan is to work at this issue at weekends. Sorry I did not made to much progress yet, I'm not too familar with c++ and with the respository. I plan finish this next weekend.

@strager
Copy link
Collaborator Author

strager commented May 17, 2021

#313 would make this task a lot easier.

@matttheus, I recommend focusing on the front-end part of this task (#258). When #313 is complete, it should be easy to connect the front-end to the back-end.

If you have progress already, please show it so I can avoid stepping on your toes.

@matttheus
Copy link
Contributor

Look here #323 please.

@strager strager linked a pull request Jun 11, 2021 that will close this issue
@strager strager linked a pull request Jun 27, 2021 that will close this issue
@strager
Copy link
Collaborator Author

strager commented Jun 27, 2021

Commit 31a88b4 added some UI for this task. It's not touch-interface-friendly, but I hope people don't try programming on their phones.

The UI doesn't show for the error documentation, but that wasn't originally part of this task.

@strager strager closed this as completed Jun 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for hire Get paid for working on this task: https://quick-lint-js.com/hiring.html
Projects
None yet
2 participants