Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Conversation

@zhixzhan
Copy link
Contributor

Description

Distinguish warning message and error message.

Task Item

refs #2261

Screenshots

Untitled

@zhixzhan zhixzhan changed the base branch from stable to master March 17, 2020 08:22
Copy link
Contributor

@a-b-r-o-w-n a-b-r-o-w-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some duplicate code paths here that I'm concerned about. Is it possible to rethink how the shell shares errors to the LU/LG fields instead of each component needing to parse/format the diagnostics?

One solution is moving the knowledge of the diagnostics into the code editors themselves because their domain is already aware of the lg/lu diagnostic.

@zhixzhan
Copy link
Contributor Author

There are some duplicate code paths here that I'm concerned about. Is it possible to rethink how the shell shares errors to the LU/LG fields instead of each component needing to parse/format the diagnostics?

One solution is moving the knowledge of the diagnostics into the code editors themselves because their domain is already aware of the lg/lu diagnostic.

I also thought about that, accept diagnostics instead of errorMsg or warningMsg can simplify a lot usage scenarios. only if we don't want customize the message, directly show the message from diagnostics. For example, same diagnostic in inline editor show less message compare to all up view, cause form have less space.
If it's not so important to customize message, we should pass diagnostic, or further more do diagnosing in editor.

@zhixzhan zhixzhan requested a review from corinagum as a code owner March 19, 2020 10:31
@zhixzhan
Copy link
Contributor Author

I worked out a solution to move to pass diagnostics:

  1. editor both accept diagnostics and errorMsg/warningMsg. but the display priority is errorMsg>diagnostics.errorMsg>warningMsg>diagnostics.warningMsg

  2. simplify the message, use L2-L3 replace line 2 - line 3, remove the source: Main . Then the message would not be too long for inline editor. but also includes necessary information.

@cwhitten cwhitten merged commit 314d4bf into microsoft:master Mar 24, 2020
@zhixzhan zhixzhan deleted the bugfix2 branch March 31, 2020 02:24
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
* lg/lu editor support warning message

* pass diagnostics to editor

* update

Co-authored-by: Andy Brown <asbrown002@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants