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

Support figure environment #3

Merged
merged 2 commits into from Feb 12, 2019

Conversation

kn1cht
Copy link
Member

@kn1cht kn1cht commented Feb 9, 2019

#1 のうち、まずはfigure環境の部分を追加しました。
普通のfigure環境と、2段組に1段組の図を挿入する時使用するfigure*環境に対応しています。

figure*は以下の部分でそのままRegExpにするとアスタリスクがメタ文字になってしまうので、先にエスケープするようにしました。

https://github.com/ta2gch/textlint-plugin-latex2e/blob/571e268eaef651dee094920409219156f1d6bf93/src/latex-to-ast/environment/common.ts#L45-L47

@@ -44,11 +44,12 @@ export const BeginEnvironment = (

export const EndEnvironment = (context: Context): Parsimmon.Parser<null> =>
Parsimmon((input, i) => {
const m = input.slice(i).match(new RegExp(`^\\\\end\\{${context.name}\\}`));
const pattern = context.name.replace(/[\\^$.*+?()[\]{}|]/g, '\\$&');
Copy link
Member

@tani tani Feb 10, 2019

Choose a reason for hiding this comment

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

現在は.*?が代入されるので問題無いのですが、BeginEnvironmentにも今後同様の可能性があるので、BeginEnvironmentの際にも同様のエスケープをしていただけますか?

Copy link
Member

Choose a reason for hiding this comment

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

また、好みの問題ですが変数の寿命が短いので変数名はpatternではなくpにしてもらえると嬉しいです。

Copy link
Member Author

Choose a reason for hiding this comment

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

ありがとうございます。対応致します。

Copy link
Member Author

Choose a reason for hiding this comment

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

  • 変数名については修正しました。
  • BeginEnvironmentのエスケープについて確認させてください。

以下の部分でBeginEnvironment(".*?", context)という形で使用されているので、同様のエスケープをすると"\\.\\*\\?"となってしまいませんか?

https://github.com/ta2gch/textlint-plugin-latex2e/blob/571e268eaef651dee094920409219156f1d6bf93/src/latex-to-ast/environment/common.ts#L55-L60

また、今回実装したfigureでも?はメタ文字として使用しているため、エスケープは使用する側に任せた方が良いのではないかと思います(引数名もpatternなので、正規表現のパターンが期待されていることは読み取れます)。

https://github.com/kn1cht/textlint-plugin-latex2e/blob/3e0bdbd5a8d20c932b6507412b3348cb6a53a15e/src/latex-to-ast/environment/figure.ts#L33-L35

Copy link
Member

Choose a reason for hiding this comment

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

全くもってその通りですね。ご指摘ありがとうございます。見た限りテストも追加されており問題無く完璧なようですので、マージさせてもらいます。

@tani tani merged commit 08afb7b into textlint:master Feb 12, 2019
@tani
Copy link
Member

tani commented Feb 12, 2019

Thank you for your contribution! cheers.

@kn1cht kn1cht deleted the feature/figure-environment branch February 12, 2019 01:48
tani pushed a commit that referenced this pull request Dec 7, 2020
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.

None yet

2 participants