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

catch type error in connection.js to partly cope with issue #10 #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kenichi-asai
Copy link
Contributor

@kenichi-asai kenichi-asai commented Apr 15, 2019

issue #10 への暫定対応として、型エラーを起こす部分を try catch で囲み、型エラーを起こした場合は undo することにしました。これで、明日の授業は乗り切ります。が、以下の問題がわかっています。

  • 以下のプログラムで、xf の body 部分に入れると(落ちることはなくなりましたが)接続を拒否された x のブロックがメインワークスペースに残ってしまいます。
let f x = ?
let test = f 0 = true
  • 上のプログラムで、x のブロックを直接、使うのではなく、スコープお砂場を開き、そこから f の body 部分に入れると落ちているようです。

完璧な commit ではないですが、将来、undo がきちんとサポートされた暁には、まあ、使えるコードになりそうなので、pull request しておきます。undo した後、型推論をし直していませんが、する必要があったら加えてください。

Copy link
Owner

@harukamm harukamm left a comment

Choose a reason for hiding this comment

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

PRありがとうございます。

型推論が失敗するかどうかのチェックは、connect_ (core.connect.js 内の関数) ではなく、接続できるかどうかをチェックする部分で行ったほうが適切かなと思います。
そうでないと、接続できることを示す黄色いハイライトが表示されているのに、接続しようとしたら接続できない、と言ったようなへんな状態が起こり得ます。

別のブランチに issue #10 fix案を作成してみました。
connect_ (core.connect.js 内の関数) ではなく、接続できるかどうかをチェックする部分で、先生の change と同じようなことをしています。エラーツールチップに関してはダミーのエラーを出力しています。
https://github.com/harukamm/ocaml-blockly/compare/mightFixIssue%2310
あまり動作確認はできていないのですが、少なくとも #10 で報告されたケースには通っています。
fetch 後、先生の手元から、ブランチ名 origin/mightFixIssue#10 で参照できると思います。

} catch (e) {
var sourceBlock = this.getSourceBlock();
var mainWorkspace = sourceBlock.workspace.getMainWorkspace();
mainWorkspace.undo(false);
Copy link
Owner

Choose a reason for hiding this comment

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

メインワークスペースは、入れ子(お砂場)になったワークスペースの根っこにある部分のワークスペースのことです。つまり、どのお砂場にあるブロックも、持っているメインワークスペースは全て一意です。
動かしたという動作を undo したいのならば、どちらかというと sourceBlock.workspace に対して undo するほうが正しいです。
(全ワークスペースで共通の undo stack を持つように実装するなら話は別ですが)
でもどっちにしても、connect_ の中でこれをやると正しく動きません。reviewに対するコメントに書きます。

@kenichi-asai
Copy link
Contributor Author

ありがとうございます!!今日の授業はこちらを使おうと思います。

型推論が失敗するかどうかのチェックは、connect_ (core.connect.js 内の関数) ではなく、接続できるかどうかをチェックする部分で行ったほうが適切かなと思います。

はい、ぼくもそう思ったのですが、connect_ を見ると、すでに繋がっていたブロックを外すなどいろいろな処理をしているので、単に connectReciprocally_ をするだけで良いのかわからなかったのです。仕方がないので、接続処理が全部終わった connect_ の最後で型を調べ、まずかったら undo という手を考えました。でも、新しいコードを見せられると、確かに接続関係だけを一時的に変更して型推論を行い、あとで戻せば大丈夫そうですね。

そうでないと、接続できることを示す黄色いハイライトが表示されているのに、接続しようとしたら接続できない、と言ったようなへんな状態が起こり得ます。

その部分は全く考えられていません。新しいコードを見て、collect というのがそれを司っているのね、というのを学びました。ありがとうございます。

別のブランチに issue #10 fix案を作成してみました。

変数を let ブロックから直接、動かす方は問題なく動いているように見えます。でも、スコープお砂場を開き、そこから変数ブロックを移動すると、黄色のハイライトになって接続できてしまい、その後、落ちてしまうようです。

@kenichi-asai
Copy link
Contributor Author

あと、mightFixIssue#10 では

という問題があるようです。この問題を解決しようと今日一日あちこちのコードとにらめっこしたのですが、解決できませんでした。上の throw している一文をコメントアウトすると落ちなくなるのですが、それは怖そうです。とりあえず、これの解決はあきらめます。

@kenichi-asai
Copy link
Contributor Author

type a_t = {a : int}
let b = {a = ?}

というブロックの ? に対して 0 のブロックを入れようとすると、つーっとブロックが流れます。遡って調べると MightFixIssue#10 の最初のコミット 3744a4f の前では起こらず、このコミットの後から起きる現象のようです。原因に心当たりはありますか。

@kenichi-asai
Copy link
Contributor Author

原因は、blocks/datatypes.jpcreate_record_typedupdateStructure の中で `initSvg' を型推論のたびに呼び出していたことのようです。これを「変更があったときのみ」呼び出すようにしたら解決したように見えます。diff は kenichi-asai@273419b にあります。

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