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

feat: basic warning mechanism #332

Merged
merged 1 commit into from
Nov 20, 2023
Merged

Conversation

hyf0
Copy link
Member

@hyf0 hyf0 commented Nov 19, 2023

@hyf0 hyf0 force-pushed the 11-19-feat_basic_warning_mechanism branch from 1019fc0 to 1b9f941 Compare November 19, 2023 13:53
@hyf0 hyf0 linked an issue Nov 19, 2023 that may be closed by this pull request
@milesj
Copy link
Contributor

milesj commented Nov 19, 2023

What's the reasoning for not using miette's built in diagnostic stuff?

@hyf0
Copy link
Member Author

hyf0 commented Nov 20, 2023

What's the reasoning for not using miette's built in diagnostic stuff?

Let me try to explain. What miette does/The main part it's allow to make diagnostics using dsl/macros. Macros has many limitations. And the expected of miette is "Write a unique error and use miette to declared a diagnostic", while we actually don't create diagnostic for each error.

In fact we currently have only one error with different error kind, this kind of breaks what miette expected.

Other than that, for supporting warnings, we need to display different contents/colors than errors, while miette is designed to work with errors and prefer macros. Maybe we could figure out a way to config it, But than I thought it's probably the same we use ariadne directly, since miette use it under the hood.

@hyf0 hyf0 force-pushed the 11-19-feat_print_fancy_messages_for_errors branch from 4ba6c75 to 40d40c1 Compare November 20, 2023 02:55
@hyf0 hyf0 force-pushed the 11-19-feat_basic_warning_mechanism branch from 1b9f941 to 7cd5f92 Compare November 20, 2023 02:55
@hyf0
Copy link
Member Author

hyf0 commented Nov 20, 2023

Merge activity

  • Nov 20, 1:16 AM: @hyf0 started a stack merge that includes this pull request via Graphite.
  • Nov 20, 1:17 AM: Graphite rebased this pull request as part of a merge.
  • Nov 20, 1:19 AM: @hyf0 merged this pull request with Graphite.

Base automatically changed from 11-19-feat_print_fancy_messages_for_errors to main November 20, 2023 06:16
@hyf0 hyf0 force-pushed the 11-19-feat_basic_warning_mechanism branch from 7cd5f92 to b4ebf32 Compare November 20, 2023 06:17
@hyf0 hyf0 merged commit 84697ce into main Nov 20, 2023
6 checks passed
@hyf0 hyf0 deleted the 11-19-feat_basic_warning_mechanism branch November 20, 2023 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for warning mechanism
3 participants