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.

Sign up for GitHub

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

Jump to bottom

Fenced block ::: foos の末尾の s が消えて class="foo" になる #41

Closed
MurakamiShinyu opened this issue Nov 4, 2020 · 16 comments
Closed

Fenced block ::: foos の末尾の s が消えて class="foo" になる #41

MurakamiShinyu opened this issue Nov 4, 2020 · 16 comments
Labels
bug Something isn't working
Milestone
v1.0.0

Comments

@MurakamiShinyu
Copy link
Member

Describe the bug

Fenced block ::: foos の末尾の s が消えて class="foo" になる

To Reproduce

入力:

::: foos
foo foo
:::

このファイルを /tmp/test.md として vfm /tmp/test.md を実行すると、次の出力となる:

$ vfm /tmp/test.md
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
</head>
<body>
<div class="foo"><p>foo foo</p></div>
</body>
</html>

class="foos" となることを期待するのに、末尾の s が消えて class="foo" になる。
名前の最後の文字が s 以外だと問題ないようだ。
（::: photos も ::: photo も同じ class="photo" という結果になる。）

（多くの条件をテストしたわけではないので、末尾の s だけが問題かどうかは未確認）。
@MurakamiShinyu MurakamiShinyu added the bug Something isn't working label Nov 4, 2020
@akabekobeko
Copy link
Member

fanced-block.ts の tokenizer 実装を単体実行して試したところ、この関数内の className は foos でした。正規表現を抜き出しても同様。よって tokenizer 呼び出し側に問題があるのだと思われます。

@AyumuTakai
Copy link
Contributor

AyumuTakai commented Dec 15, 2020 •
edited
Loading

気になったので実験してみました。

::: foos
foo foo
:::

をtest.mdという名前で保存したあと

masterブランチとdependabot/npm_and_yarn/node-fetch-2.6.1ブランチの最新を取得して

yarn build
npm install -g .
vfm test.md

をそれぞれ実行したところ、どちらも以下の出力が得られました。

<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
</head>
<body>
<div class="foos"><p>foo foo</p></div>
</body>
</html>

正しい結果が得られているように見えます。

vivliostyle-cliで既存原稿に

::: foos
foo
:::

を追加してbuildしたところ、該当箇所に以下が出力されました。こちらは不具合が出ています。

<div class="foo"><p>foo</p></div>

package.jsonを見たところ、使っているvfmのバージョンは1.0.0-alpha.10でした。

@AyumuTakai
Copy link
Contributor

AyumuTakai commented Dec 15, 2020 •
edited
Loading

vivliostyle-cliにインストールされている@vivliostyle/vfm/lib/plugins/fenced-block.jsを確認したら、正規表現が以下のものになってました。

s*(.*?)s*\n([\w\W]+?)\n

空白文字のエスケープがただのsになってしまっていて、ふたつめのs*にマッチしてしているようです。

@AyumuTakai
Copy link
Contributor

AyumuTakai commented Dec 15, 2020 •
edited
Loading

gitのログを確認したところ
ad3bace
で修正されてました。

@akabekobeko
Copy link
Member

調査ありがとうございます。内容をみるに現状の VFM をリリースすれば ad3bace が反映されて本件は修正されそうですね。

@AyumuTakai
Copy link
Contributor

そうですね。

今までは幸い不具合に当りませんでしたがフェンスドブロックを使いまくっていてるのでリリースして頂けると助かります。

よろしくお願いいたします。

@akabekobeko
Copy link
Member

@MurakamiShinyu

前述の通り本件はリポジトリー上で修正されていますが、現時点でリリースされている VFM 1.0.0-alpha.10 には未反映のようです。今後のリリースをどのようにするかは検討するとして、未反映の修正や変更はひととおりリリースしてからのほうが開発再開するうえでわかりやすいと考えています。

また私がリリース作業可能であること、もし失敗するようであればなにが必要なのかを調べるため、リリースを実行してみてよいでしょうか？リリース手順についてはリポジトリーの CONTRIBUTING.md に記載されている Bump pre-release version を試す予定です。

@akabekobeko
Copy link
Member

あと Issue Template の Bug report を変更して VFM と Node.js バージョンを明記してもらえるようにしました。close されず残っている他の Issue も本件と同様に対応済みかもしれませんし、その場合、どのバージョンが対象かを明示することは安全な議論に寄与するでしょう。

@MurakamiShinyu
Copy link
Member Author

@akabekobeko

リリースを実行してみてよいでしょうか？リリース手順についてはリポジトリーの CONTRIBUTING.md に記載されている Bump pre-release version を試す予定です。

はい、お願いします。

@akabekobeko
Copy link
Member

リリースを試しましたがエラーになりました。npm login は実行済みで個人 npm は publish できる環境なので @vivliostyle/vfm に対する権限がないのだと思われます。

$ yarn release:pre
yarn run v1.22.10
$ release-it --preRelease --npm.tag=next
ERROR Not authenticated with npm. Please `npm login` and try again.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@akabekobeko
Copy link
Member

@MurakamiShinyu

npmjs 上の私のアカウントは akabeko なので @vivliostyle (おそらく organization) で許可をお願いします。許可されたら明日以降に再試行します。

@akabekobeko
Copy link
Member

akabekobeko commented Dec 15, 2020 •
edited
Loading

Slack にて村上さんより権限はあるとの情報を得たので再試行したが同じエラーになった。本リポジトリーや構成ファイルの設定を調査する。

@akabekobeko akabekobeko mentioned this issue Dec 15, 2020
Closed
@akabekobeko
Copy link
Member

akabekobeko commented Dec 16, 2020 •
edited
Loading

@MurakamiShinyu @AyumuTakai
現状の実装を 1.0.0-alpha.11 としてリリースしてみました。私の実施した手順や必要な設定は #42 に記録しています。pre だと npm@next 扱いになるため、これを cli に組み込むなら package.json で直に指定するか npm i @vivliostyle/vfm@next でインストールする必要があるようです。

なお #42 にも書きましたが yarn release:pre だとダメで npm run release:pre なら私の環境でもリリース可能となりました。yarn のほうがダメな理由はエラー メッセージに npm login を実行せよ的な情報があるので npmjs のアカウント情報が参照されていないのだと思われます。

@akabekobeko
Copy link
Member

通常の npm としては alpha.6 で止まっています。リリースにおける pre をどうするかは課題ですね。今回はお試しということもあって pre にしましたが、alpha とつけているならいっそ通常リリースのみにするのもありでしょうか。

@MurakamiShinyu
Copy link
Member Author

VFMはまだalpha版しか存在しないのだから、latestタグを削除して、1.0.0が完成するまではnextタグだけにするというのはどうでしょう？ alpha.6 にlatestタグがついているけどそれがstableというわけでないでしょうから。
npm dist-tag rm @vivliostyle/vfm latest かな。

@akabekobeko akabekobeko mentioned this issue Jan 10, 2021
Closed
9 tasks
@MurakamiShinyu
Copy link
Member Author

リリースのtagについては、開発者会議(2021-01-09)で "next" (release:pre) を使うのをやめる方針に決まったのでこの議論も終了。このissueは閉じてよいですね。

@akabekobeko akabekobeko added this to the v1.0.0 milestone Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
v1.0.0
Development

No branches or pull requests
3 participants
@akabekobeko @AyumuTakai @MurakamiShinyu