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

Better formatting of custom errors #8140

Closed
Reinmar opened this issue Sep 24, 2020 · 9 comments · Fixed by #8218
Closed

Better formatting of custom errors #8140

Reinmar opened this issue Sep 24, 2020 · 9 comments · Fixed by #8218
Assignees
Labels
type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Reinmar
Copy link
Member

Reinmar commented Sep 24, 2020

📝 Provide a description of the improvement

Right now an error prints like this:

It was formatted slightly better in the past because there was a period after the message

We don't have the message now but we have to separete "Read more" from the error code somehow. Some ideas:

  • CKEditorError: error-code | Read more: URL
  • CKEditorError: [error-code] Read more: URL
  • CKEditorError: error-code -> Read more: URL
  • Or something without "Read more" at all

If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@Reinmar Reinmar added the type:improvement This issue reports a possible enhancement of an existing feature. label Sep 24, 2020
@Reinmar
Copy link
Member Author

Reinmar commented Sep 24, 2020

cc @jodator

@jodator
Copy link
Contributor

jodator commented Sep 24, 2020

Another thing to improve - error with params.

@jodator
Copy link
Contributor

jodator commented Sep 24, 2020

And check the test for proper format: #8124 (comment).

@Reinmar
Copy link
Member Author

Reinmar commented Sep 24, 2020

Another thing to improve - error with params.

I wonder if we could learn something from other projects here. Maybe there are some tricks that could improve the readability even further.

BTW, one of these things would be if we were able to shorten the URL as it obscures the view... But I guess this is not easily doable.

@jodator
Copy link
Contributor

jodator commented Sep 24, 2020

Custom URL shortener 😉 https://www.youtube.com/watch?v=Z57566JBaZQ. 

But we could check how to improve in other repos for sure.

@jodator
Copy link
Contributor

jodator commented Sep 29, 2020

I've played a bit with this - I'd go with a two-line message:

error-short-id { param: 'value" } 
Read more: https://ckeditor.com/docs/ckeditor5/latest/.../#error-error-short-id 

console.log:

Selection_046

CKEditorError:

Selection_048

👍  Pros:

  • Link to documentation is always in the same position, so the "Read more" call to action stands out.
  • Optional params are close to the error id.

👎 Cons:

  • requires a new method for console logging errors to have consistent error log formatting

@jodator
Copy link
Contributor

jodator commented Sep 30, 2020

It seems that 3 👍 is enough :) Thanks!

@Reinmar
Copy link
Member Author

Reinmar commented Oct 6, 2020

Did you create a PR for this? I can see it's on review but I can't find any linked PR.

@jodator
Copy link
Contributor

jodator commented Oct 6, 2020

Did you create a PR for this? I can see it's on review but I can't find any linked PR.

OMG Sorry - I was 100% sure that the PR was created because I've updated the PR in eslint-plugin-ckeditor5-rules repository (ckeditor/eslint-plugin-ckeditor5-rules#1). PR is ready here: #8218.

Reinmar added a commit that referenced this issue Oct 14, 2020
Other (utils): Make custom errors more readable in the console. Closes #8140.

MINOR BREAKING CHANGE (utils): The `attachLinkToDocumentiation()` helper was removed. To log errors to the console with attached link to the documentation use `logWarning()` and `logError()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants