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

ChordWiki用の自動転調機能を追加(2) #64

Merged
merged 24 commits into from
Sep 6, 2021

Conversation

ts-uc
Copy link
Contributor

@ts-uc ts-uc commented Sep 6, 2021

revertするのが難しそうだったので、新しいブランチを立ち上げた上で古いブランチから toICN-after.js だけをマージし、新たにpull requestを立ち上げました。

@inajob
Copy link
Owner

inajob commented Sep 6, 2021

お手数おかけします;

@inajob
Copy link
Owner

inajob commented Sep 6, 2021

sh gen.shを実行した結果もcommitしてほしいです。

toICN-debug.js Show resolved Hide resolved
}
}
else{
let icn = module.exports(""+e.firstChild.nodeValue);
Copy link
Owner

Choose a reason for hiding this comment

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

少しリファクタリングして構造が変わっています。

Suggested change
let icn = module.exports(""+e.firstChild.nodeValue);
let icn = exports.toICN(""+e.firstChild.nodeValue);

@@ -36,10 +39,13 @@ if(detectedKey == ""){
key = "";
detectedKeyMinorSignature = "u";
}
else{
isKeyWritten = true;
}

let displayedKey = exports.DisplayedKey(detectedKey, detectedMinorSignature);
Copy link
Owner

Choose a reason for hiding this comment

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

すいませんmainにバグが混入しています・・このPRで直してもらえると・・

Suggested change
let displayedKey = exports.DisplayedKey(detectedKey, detectedMinorSignature);
let displayedKey = exports.DisplayedKey(detectedKey, detectedKeyMinorSignature);

@ts-uc
Copy link
Contributor Author

ts-uc commented Sep 6, 2021

  • リファクタリングに対応しました
  • isKeyWrittenを用いた方法に書き換えました
  • それに伴って、test.jsにも変更を加えました
  • 指摘されたバグを修正しました

src/toICN-after.js Outdated Show resolved Hide resolved
ts-uc and others added 2 commits September 6, 2021 10:53
Co-authored-by: inajob <kinadu@zlab.co.jp>
src/toICN-after.js Outdated Show resolved Hide resolved
ts-uc and others added 2 commits September 6, 2021 10:57
Co-authored-by: inajob <kinadu@zlab.co.jp>
let keyMinorSignature = "";
let detectedKey = "";
let detectedKeyMinorSignature = "";
//ChordやKeyを読む
let chordElms = [];
let keyElm;
let isKeysOnChordElms = false; //これがtrueの場合は、chordElmsにkeyも含まれるようになる
Copy link
Owner

Choose a reason for hiding this comment

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

let isKeyWritten = false;

的なものがここに無いと後続でエラーになることがあります。

Copy link
Owner

Choose a reason for hiding this comment

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

具体的には Keyが明示されていない時 isKeyWritternが定義されなくて、後続で isKeyWritten is not defined となります。
例えば https://www.ufret.jp/song.php?data=41 でこのブックマークレットが動作しません。

@ts-uc
Copy link
Contributor Author

ts-uc commented Sep 6, 2021

以下の変更を加えました。

  • ChordWikiのときは、chordElmsにchordを、keyChordElmsにkeyとchordを入れる(従来: keyChordElmsにkeyとchordを入れる)
  • toICN-after.js の下の方に引き渡すelementを、条件分岐でchordElmsかkeyChordElmsを切り替える

@inajob
Copy link
Owner

inajob commented Sep 6, 2021

%12つなげるとだめなんですね・・ 悩ましいな・・ひとまずスペースを入れる対応で・・

src/toICN-after.js Outdated Show resolved Hide resolved
ts-uc and others added 2 commits September 6, 2021 11:33
Co-authored-by: inajob <kinadu@zlab.co.jp>
@inajob
Copy link
Owner

inajob commented Sep 6, 2021

あと #64 (comment) の問題ですね。

@ts-uc
Copy link
Contributor Author

ts-uc commented Sep 6, 2021

その他に、なぜか転調がうまくいかなくなる不具合が発生しました

@ts-uc
Copy link
Contributor Author

ts-uc commented Sep 6, 2021

image
「キーは認識されているけどそれが反映されない」状態になってしまいました

@inajob
Copy link
Owner

inajob commented Sep 6, 2021

https://github.com/inajob/toICN/pull/64/files#diff-df9afdd2d6f0be75d254c910806794ff16c81c87d4e849021a5665e8306cab9eR87
をコメントアウトすると一見うまくいってますね・・(ただし表示上は変化しなくなりますが・・)

なんだろうこれ

toICN-debug.js Outdated Show resolved Hide resolved
ts-uc and others added 2 commits September 6, 2021 17:16
Co-authored-by: inajob <kinadu@zlab.co.jp>
@ts-uc
Copy link
Contributor Author

ts-uc commented Sep 6, 2021

直ってます直ってます!素晴らしい

@ts-uc
Copy link
Contributor Author

ts-uc commented Sep 6, 2021

いや、直ってない

@ts-uc
Copy link
Contributor Author

ts-uc commented Sep 6, 2021

変更する箇所を間違えました。修正します。

@inajob
Copy link
Owner

inajob commented Sep 6, 2021

あ、自分の指摘した箇所がファイル違いましたね。でもおそらく・・

@ts-uc
Copy link
Contributor Author

ts-uc commented Sep 6, 2021

image
今度こそ直りました!

@inajob
Copy link
Owner

inajob commented Sep 6, 2021

途中転調の曲は処理がちょっと複雑なので 後で良いのでテストを入れたいですね。

@ts-uc
Copy link
Contributor Author

ts-uc commented Sep 6, 2021

マージしました。

@inajob
Copy link
Owner

inajob commented Sep 6, 2021

良さそうなのでマージします!
転調が実装できたのすごい!

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