Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Use console.warn and console.error #1336

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

Open
atjn wants to merge 5 commits into amark:master
base: master
Choose a base branch
Loading
from atjn:warnings
Open

Use console.warn and console.error #1336

atjn wants to merge 5 commits into amark:master from atjn:warnings

Conversation

Copy link
Contributor

@atjn atjn commented Aug 21, 2023
edited
Loading

A lot of the core Gun code uses console.log for warnings and errors. This PR uses the appropriate console.warn and console.error methods.

This helps with readability in the console, because messages are styled differently depending on their severity. It also helps when using custom debuggers, as you can now filter between log severities.

It is possible that I have used the wrong severity level for a few logs, but I think this is a big step in the right direction, and something that can easily be tweaked when you find those misses :)

draeder reacted with thumbs up emoji
@atjn atjn marked this pull request as draft August 21, 2023 08:34
@atjn atjn marked this pull request as ready for review August 21, 2023 09:11
Copy link
Collaborator

some of us use custom js environments that don't have .warn ? and is why it has .log as a fallback?

this is the top reason why the pr cant be pulled

Copy link
Contributor Author

atjn commented Dec 29, 2023

some of us use custom js environments that don't have .warn ? and is why it has .log as a fallback?

@bmatusiak I was not aware of this, can you point me to some documentation on the environment that doesn't support warn? I have never heard of missing warn functionality. It is part of the original console specification, and it is very easy to just alias log as warn, so I never expected an environment to be missing support.

Copy link
Collaborator

https://www.espruino.com/Reference#console

true,, it could be aliased, but with embedded devices need less code because they only have so much storage

Copy link
Contributor Author

atjn commented Jan 2, 2024

This issue should be fixed in the next release of Espruino, see:
Issue: espruino/Espruino#1302 (comment)
Commit: espruino/Espruino@08fca8d

Do you have any other custom runtime I should look into?

Copy link
Collaborator

nice!. will re-review when the time comes

atjn reacted with thumbs up emoji

Copy link
Contributor Author

atjn commented Jan 10, 2024

@bmatusiak This is now released in Espruino 2v20.
Docs: https://www.espruino.com/Reference#t_l_console_warn

Copy link
Owner

amark commented Feb 7, 2024

oye, I guess if even espruino supports it... that's :P probably ticks a lot of boxes (I'm like @bmatusiak on these matters too). Tho @bmatusiak doing the labor of love of testing/fixing cross-env support, so if he thumbs up I'll pull.

Copy link
Contributor Author

atjn commented Feb 7, 2024

I just rebased the code. Looking forward to your review @bmatusiak :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /