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

Display error messages in the wasm demo #31

Merged
merged 3 commits into from
Oct 27, 2023

Conversation

ppputtyo
Copy link
Contributor

@ppputtyo ppputtyo commented Oct 26, 2023

image

概要

wasm版demoにおいて、フォーマットに失敗した際にエラーメッセージを表示するように変更しました。 (#25 )
ただし、UroboroSQLFmtErrorを単純に文字列に変更して表示しているため、エラー内容はわかりにくいものとなっています。
エラーメッセージの内容については別で議論する必要があると思うので、とりあえずドラフトの形でPRを作成しました。

実装

以前はformat_sql_for_wasmでフォーマット結果を格納したポインタを返していました。
しかし、今回の変更ではフォーマット結果とエラーメッセージの2つの文字列を返したいため、実装を以下のように変更しました。

  1. Rustであらかじめ結果とエラーメッセージを格納するメモリを確保 (それぞれRESULTERROR_MSGとする。現状大きさ50000としている。)
  2. wasmの初期化時にRESULTERROR_MSGの先頭アドレスを取得 (Rust側でアドレスを教える関数を作成して実現)
  3. format_sql_for_wasmが呼び出されたらフォーマットを実行し、正常ならば結果をRESULTに、エラーの場合はエラーメッセージをERROR_MSGに格納
  4. js側でRESULTERROR_MSGから結果とエラーメッセージを取得して表示

@ppputtyo ppputtyo requested a review from tanzaku October 26, 2023 05:06
@tanzaku
Copy link
Collaborator

tanzaku commented Oct 27, 2023

ありがとうございます!

ただし、UroboroSQLFmtErrorを単純に文字列に変更して表示しているため、エラー内容はわかりにくいものとなっています。
エラーメッセージの内容については別で議論する必要があると思うので、とりあえずドラフトの形でPRを作成しました。

そうですね、エラーメッセージの内容はまだわかりにくいので改善していけたらいいと思います。
ただ、wasm版でエラーメッセージが出せるようになるだけで大きな改善なので、まずはエラーメッセージの改善なしにマージしてしまっていいのではと思います。

メッセージの改善については一旦別issueに切り出しました。 #32

.github/workflows/CI.yml Show resolved Hide resolved
crates/uroborosql-fmt-wasm/src/lib.rs Outdated Show resolved Hide resolved
@ppputtyo ppputtyo marked this pull request as ready for review October 27, 2023 09:15
@tanzaku
Copy link
Collaborator

tanzaku commented Oct 27, 2023

LGTM!

@tanzaku tanzaku merged commit 9d377fb into main Oct 27, 2023
8 checks passed
@tanzaku tanzaku deleted the dev_kawabuchi_show_error_in_wasm branch October 27, 2023 10:11
@ppputtyo ppputtyo mentioned this pull request Oct 28, 2023
@ppputtyo ppputtyo restored the dev_kawabuchi_show_error_in_wasm branch October 28, 2023 03:06
@ppputtyo ppputtyo deleted the dev_kawabuchi_show_error_in_wasm branch October 28, 2023 03:08
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.

2 participants