Skip to content

Conversation

shirayu
Copy link
Contributor

@shirayu shirayu commented Dec 29, 2018

#7 の一部に対処しました.

  • "することを可能"
  • "することをできる"

の検出を

  • "すること[助詞]可能"
  • "すること[助詞]できる"

に拡充しました.

またREADMEも修正しました.

#6 と同様にtechnological-book-corpus-jaを使った比較結果も添付します.

Copy link
Member

@azu azu left a comment

Choose a reason for hiding this comment

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

ありがとうございます。
コメントしました

src/index.js Outdated
const dictionaryList = require("./dictionary");
const createMatchAll = require("morpheme-match-all");
const replaceWithCaptureTokens = (text, tokens, actualTokens) => {
const replaceWithCaptureTokens = (text, tokens, actualTokens, for_expected) => {
Copy link
Member

@azu azu Dec 31, 2018

Choose a reason for hiding this comment

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

引数が増えてきたので引数をオブジェクトにした方がよさそうですね。
後、expectedとmessageを関数として分けたほうがいい気がしますね。
(内部で分岐が増えると読みにくくなると思うので)

  • token -> matchTokens
const createExpected = ({ text, matcherTokens, actualTokens }) => {
}
const createMessage = ({ text, matcherTokens, actualTokens }) => {
}
// 共通なのはtoken単体に対する置換処理にできそうな気がする
const replaceTokenWith = (matcherToken, actualToken) => {
/* この辺だけのtoken単体に対する置換処理を切り出せる?
        let to;
        if (for_expected && token["_capture_to_expected"]) {
            to = token["_capture_to_expected"](actualTokens[index]);
        } else if (!for_expected && token["_capture_to_message"]) {
            to = token["_capture_to_message"](actualTokens[index]);
        } else {
            to = actualTokens[index].surface_form;
        }
}*/
}


createExpected({ 
  text: matchResult.dict.expected,
  matcherTokens: matchResult.dict.tokens,
  actualTokens: matchResult.tokens 
});

のようなイメージ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

なるほど,検討してみます!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pushしました

src/index.js Outdated
if (for_expected && token["_capture_to_expected"]) {
to = token["_capture_to_expected"](actualTokens[index]);
} else if (!for_expected && token["_capture_to_message"]) {
to = token["_capture_to_message"](actualTokens[index]);
Copy link
Member

Choose a reason for hiding this comment

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

これ利用していますか? 辞書にはない気もします。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

これは
75eb8bb#diff-d460974cf13fda7ff6679698d7b13fd4R84
で使っています

Copy link
Member

@azu azu Dec 31, 2018

Choose a reason for hiding this comment

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

_capture_to_message の方はない気がします?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

はい, _capture_to_message は記述を統一的にするために付けただけで,今回のPRでは使っていません

Copy link
Member

@azu azu left a comment

Choose a reason for hiding this comment

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

LGTM

@azu azu merged commit 882c2bb into textlint-ja:master Dec 31, 2018
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