Skip to content

Operatorの命令ブロックをRubyに変換できるようにしました #68

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

Merged

Conversation

t-kazu
Copy link
Collaborator

@t-kazu t-kazu commented Oct 19, 2018

命令ブロック Ruby
screenshot from 2018-10-19 10-45-36 screenshot from 2018-10-19 10-45-51
  • 含まれるのブロックについて
    screenshot from 2018-10-19 10-29-35
    Rubyのコード的に ${str1}.include(${str2})? にしましたが、${str1}.contains(${str2})? の方がよろしいでしょうか?

@takaokouji
Copy link

@t-kazu レビューしました。お手数ですが、修正をお願いします。
優先順位の値は、
smalruby-editor/app/assets/javascripts/blocks/operators.js.coffee.erb
https://github.com/smalruby/smalruby-editor/blob/master/app/assets/javascripts/blocks/operators.js.coffee.erb
が参考にできます。

@t-kazu t-kazu force-pushed the feature/rubygenerator-for-operator branch from fd49e4c to c89f3f0 Compare October 19, 2018 08:09
@t-kazu t-kazu force-pushed the feature/rubygenerator-for-operator branch from c89f3f0 to 3b43e06 Compare October 19, 2018 08:12
Copy link
Collaborator Author

@t-kazu t-kazu left a comment

Choose a reason for hiding this comment

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

優先順位のことすっかり忘れていました。修正しました。
ご確認お願い致します。

};

Blockly.Ruby.operator_and = function (block) {
const operand1 = Blockly.Ruby.valueToCode(block, 'OPERAND1', Blockly.Ruby.ORDER_NONE) || 'false';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

何も無い時はfalseの動きだったので、何も無い時はfalseを返すようにしています。

@takaokouji
Copy link

LGTM

@takaokouji takaokouji merged commit a311080 into smalruby:develop Oct 19, 2018
@t-kazu t-kazu deleted the feature/rubygenerator-for-operator branch October 22, 2018 04:57
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