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

Redo "内部リンクを相対リンクで生成する機能 with settings.USE_RELATIVE_LINK = True" #81

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

akinomyoga
Copy link
Member

@akinomyoga akinomyoga commented Jun 30, 2022

#80 で一旦 revert された #78 をやり直し。

cpprefjp/site#917 (comment) で言及されたものです。これも複数のリポジトリに変更があります。cpprefjp/site#977 のデモとして御覧頂いた https://akinomyoga.github.io/cpprefjp-site/ はこの PR の実証も兼ねていました (従来だとそもそも cpprefjp はサイトのトップレベル以外に配置できなかったことに注意)。取り敢えずのところ私的に一通り気になるところは対応しました。

背景

現状では site_generator で生成されるコンテンツはトップレベル / に置かれるという想定でリンクが生成されています。このためローカルで閲覧しようとすると毎回新しいWebサーバーを立ち上げる必要が出てきます (特にリモートのマシン上で変換しているとファイヤーウォールやSELinuxやプロキシなどの設定もしなければならずとても面倒)。

内部のリンクを相対リンクにすればローカルで単にブラウザで開いて閲覧することができるので便利。また、単に設定済みの Web サーバーのサブディレクトリに配置すれば閲覧できるようになることを期待。未だ簡単なテストのみしかしていません。

変更 (2022-10-10 更新)

現状の課題

対応済: navbar.cssbackground-image: url(.../cpprefjp-icon-v1.1-transparent.png)

→ data スキームによる埋込に対応しました。cpprefjp/static/static/original-icons/cpprefjp-icon-v1.1.svgにSVGがあったので、それを data スキームで埋め込んでみました。

対応済: ローカルで閲覧しようとすると crsearch.json の読み込みで CORS (Cross-origin resource sharing) エラー

今は file:// の時に (JSON through XHR の代わりに) JSONP through <script> を使うようにしています。

対応済: crsearch.json/run.py にハードコードされている base_url を何とかする

→現状は取り敢えずハードコートされている base_url はデフォルトの base_url と思うことにして、それとは別に CRSearch の初期化オプションで base_url を動的に指定できるように拡張しています (d1eb6a47 @ crsearch, 852c5074 @ kunai)。

対応済: raw.githubusercontent.com/cpprefjp/image にあるデータもサイトの中に含めるべきでは

これでインターネットに繋がっていない環境でもローカルの cpprefjp で図が見られます。
あと、images の中の図に対するリンクの指定も標準化しています。

対応済: manifest.json の位置指定が変

これは元々あった問題ですがリンク関係ということでこのPRに含めてしまっています b99a8c2

@yumetodo
Copy link
Member

改めまして

現状 site, kunai, crsearch の出力結果を cpprefjp.github.io に全部くっつけて入れているのだから、image のデータも deploy 時に cpprefjp.github.io の中に入れて良いのではないかということです (実際 Web ページの画像を開発用 repository の一つである cpprefjp/image から raw.githubusercontent.com を通して直接持ってくるのは何か変な感じがする)。

これ改めて考えるとrepoの肥大化を防ごうとした説ないですかね

で、今となってはsubmoduleとして画像置けばいいのではないかって思います。
GitHub PagesにSubmoduleを使う時の注意点 - Qiita

@akinomyoga
Copy link
Member Author

akinomyoga commented Jun 30, 2022

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

改めまして

現状 site, kunai, crsearch の出力結果を cpprefjp.github.io に全部くっつけて入れているのだから、image のデータも deploy 時に cpprefjp.github.io の中に入れて良いのではないかということです (実際 Web ページの画像を開発用 repository の一つである cpprefjp/image から raw.githubusercontent.com を通して直接持ってくるのは何か変な感じがする)。

これ改めて考えるとrepoの肥大化を防ごうとした説ないですかね

そう。その説は私も頭を過ったのですが、実際のところ画像は一回追加したら基本的に変更されないし、それよりはキャッシュ防止の為の URL hash (cachebust) の方が断然サイズを食っている (例えば cpprefjp/cpprefjp.github.io@b06298b を見て下さい! (フリーズ注意。スマホでは開かない方が良いかも)) ので、画像サイズ程度は気にすることではないなぁと思いました。

で、今となってはsubmoduleとして画像置けばいいのではないかって思います。
GitHub PagesにSubmoduleを使う時の注意点 - Qiita

うーん。今の話は生成結果に images のデータを含めるということであって、site などのリポジトリの中に画像を含めるという話ではない…と思うのですがそういうことではない? 別の言い方をすれば、現状ではユーザーがページを閲覧した時に https://cpprefjp.github.io/ のサイト外から画像を読み込む形になっているが、画像も https://cpprefjp.github.io/ の下に配置したいという話です。


GitHub PagesにSubmoduleを使う時の注意点 - Qiita

追記 あぁ。なるほど。GitHub Pages で submodule を使うことができるんですね。。

でも今回はローカルで生成した時にも反映させたいので gh-pages 特有の効果には頼りたくないというのと、GitHub のリポジトリの内容が含まれる .zip のリンク (例 https://github.com/akinomyoga/cpprefjp-site/archive/refs/heads/gh-pages.zip) からダウンロードしたものを直接閲覧できたらいいな、というのもあるのでやはり cpprefjp.github.io に含めたいなぁと思っています。

@yumetodo
Copy link
Member

yumetodo commented Oct 9, 2022

だいぶ塩漬けにしてしまいました。これなんだっけと思い出すところから・・・

毎回新しいWebサーバーを立ち上げる必要が出てきます

そもそもWebさーばーごとき、立ち上げればいいし、リモートのものをっていうならssh portforwardingないしはVSCode Serverのport forwardで間に合ってるのでうーんってなったんですが、

GitHub のリポジトリの内容が含まれる .zip のリンクからダウンロードしたものを直接閲覧できたらいいな

これ言われてなるほどなと思いました。 cpprefjp/site#917 でそういう需要は言われてましたもんね・・・。とかいたところで、そもそもこの話が cpprefjp/site#917 から来ていたことを思い出しました。

モチベーションについては理解しました。同時に

image のデータも deploy 時に cpprefjp.github.io の中に入れて良いのではないか

に対する懸念点もなくなりました。

@yumetodo
Copy link
Member

yumetodo commented Oct 9, 2022

@akinomyoga あと課題点ってなんでしたっけ

@akinomyoga
Copy link
Member Author

@akinomyoga あと課題点ってなんでしたっけ

特に個人的に残っている課題点はなくもう満足してます。

@yumetodo
Copy link
Member

yumetodo commented Oct 9, 2022

それでは自分は基本設計としては異論ないので実装を眺めることにします

@akinomyoga
Copy link
Member Author

ありがとうございます

@yumetodo
Copy link
Member

yumetodo commented Oct 9, 2022

@akinomyoga 説明欄のdiffのリンク先にキーワード自動リンクの差分が混ざっていたり、レビュー記録が残し辛いので各repoでPRを切っていただきたいです。

@akinomyoga
Copy link
Member Author

OK. これは diff のリンクが昔の commit との比較になっているからですね。元々完全に棄却されるかもしれないと思ってPRを無駄に開かないようにしてましたが、それぞれ PR にします

@yumetodo
Copy link
Member

yumetodo commented Oct 10, 2022

image

手元にDockerがいなかったので適当なLinux VPS環境で実施しました。
crsearchはnpm packしてkunaiでnpm install <npm packでできたtarball>して差し替えて動かしました。sudo ./docker.sh run settings.cpprefjp_relativeしてビルドが通り、成果物をrsyncで手元に取ってきて手元のFirefoxにD&Dしたところ、ひとまず動いていそうです。

この後デグレがないか、boostjpの動作確認など検査することになります。

確認した観点は以下です。他の検証観点があれば教えてください。

  • cpp exampleと記載されたコードブロックの実行機能が動作すること
  • imagesにおいている画像が埋め込まれていること
  • 検索が動作すること
  • google検索への遷移機能が動作すること
    • <keyword> site:に展開されるので正しく検索できない。ただしそういうものといえばそうとみなすこともできる
    • ただ、kunaiでせっかく document.querySelector('meta[name="twitter:url"]') || document.querySelector('meta[property="og:url"]');とかしているのだから惜しい気はする
  • 定義語自動リンク: プロセッサ markdown_to_html.defined_words を追加 #79 が動作すること

@akinomyoga
Copy link
Member Author

  • google検索への遷移機能が動作すること
    • <keyword> site:に展開されるので正しく検索できない。ただしそういうものといえばそうとみなすこともできる
    • ただ、kunaiでせっかく document.querySelector('meta[name="twitter:url"]') || document.querySelector('meta[property="og:url"]');とかしているのだから惜しい気はする

御指摘ありがとうございます。これは対応いたします。ただ、今忙しいので2週間程待っていただくことになるかもしれません。

This reverts commit e4597df, which was
actually the reverse commit for the change
61156b4 made to
af55908.
@akinomyoga
Copy link
Member Author

akinomyoga commented May 17, 2024

[ ] google検索への遷移機能が動作すること

  • <keyword> site:に展開されるので正しく検索できない。ただしそういうものといえばそうとみなすこともできる
  • ただ、kunaiでせっかく document.querySelector('meta[name="twitter:url"]') || document.querySelector('meta[property="og:url"]');とかしているのだから惜しい気はする

@yumetodo 2週間ではなく2年間経ちましたが (すみません)… akinomyoga/cpprefjp-kunai@5fc7babakinomyoga/cpprefjp-crsearch@91b3583 で対応しました。

追記: もし手元に以前の環境を未だお持ちでしたら差分更新は "たぶん" ↓で行けます (駄目だったらすみません)。もし以前の環境がなければ site_generator の README にある説明を一から。

$ cd /path/to/crsearch
$ npm pack
$ cd /path/to/site_generator/kunai
$ npm install <path/to/cresarch-3.0.23.tar.gz generated by the above npm pack>
$ cd /path/to/site_generator
$ ./kunai/docker.sh install
$ ./kunai/docker.sh dist
$ ./crsearch.json/docker.sh run
$ ./docker.sh run settings.cpprefjp_relative
$ ls cpprefjp/cpprefjp.relative

追記: 一から構築する時は README に追記している追加手順 が必要でした。

@akinomyoga
Copy link
Member Author

akinomyoga commented May 18, 2024

テストする側で生成環境を整えるのも大変だと思いますので、生成結果を以下に添付します。Zip を使う場合はローカルで展開して index.html をブラウザで開いていただければそのまま御覧になれます。

留意事項

上で既に言及していますが今来た人のために。

  • この変更によって 相対リンクで生成する機能を off にしていても、cpprefjp/image 内部の画像ファイル (.png, .jpg, .svg) も (raw.githubusercontent.com の外部リンクではなく) Web サイトの中に含める様に変更しています。但しそれ以外のファイル (.tsv.tsv.7z) は外部リンクのままです。つまり、cpprefjp/cpprefjp.github.ioimage 由来の画像が追加されます。

@faithandbrave
Copy link
Member

ありがとうございます。いい感じですね。
こちら残タスクはまだありますか?

@akinomyoga
Copy link
Member Author

akinomyoga commented May 24, 2024

取り敢えず一区切りはついています! ただし、 @yumetodo さんが今週末に見たいということを Twitter で言及されていました。

あと、この機能は機能として実装しただけで、既存の settings.cpprefjp を置き換えるものではありません。使うにはそれぞれ site_generator を走らせる時に settings.cpprefjp の代わりに settings.cpprefjp_relative を指定する必要があります。

cpprefjp/site#917 にあるような物を提供するには、更に CI の生成を settings.cpprefjp_relative に置き換えるか、別枠で CI を走らせて Zip に固めるか、誰かが手元で生成して手動でアップロードするかなどが必要です。それはそれで (そもそも提供するかも含めて) また議論が必要になるような気がするので、分けておきたいと思っています。

@akinomyoga
Copy link
Member Author

Google 検索の site: 対応のために新しく追加した変更について、変数名は project_url ではなくて online_url の方がいいかもしれないと思われてきました。

@faithandbrave
Copy link
Member

とくに不都合がなければcpprefjp_relativeをデフォルトにしてもいい気がしますが、問題ありそうでしょうか?
リリース物を自動で作れるかと、最新リリースのzipへのリンクを取得できるかなど (バージョン指定ではなくlatestみたいなの)、一旦調べないとですね。

@akinomyoga
Copy link
Member Author

akinomyoga commented May 27, 2024

お返事遅くなりすみません。

crsearch オプション名変更

Google 検索の site: 対応のために新しく追加した変更について、変数名は project_url ではなくて online_url の方がいいかもしれないと思われてきました。

こちら修正しました。

manifest.json で相対パスを使用

テストしていて気付いたので追加修正です。

340f778

cpprefjp_relative を既定にする案

とくに不都合がなければcpprefjp_relativeをデフォルトにしてもいい気がしますが、問題ありそうでしょうか?

分かる範囲では特に問題はないと思っています (細かい問題が出てくるかどうかは実際に試してみないと分からないかもしれませんが、原理的な問題はないと考えています)。

追記: と思ったのですが、メインのサイト https://cpprefjp.github.io 向けの設定だと Google アナリティクス関連のコードが埋め込まれる一方で、現在の相対リンク版ではそれを埋め込まないようにしています (これは settings/cpprefjp_relative.py の中で指定していますので変更可能です)。例えば、登録していないURLの場合は Google アナリティクス側が適当に無視してくれることを期待して、メインサイト版・相対リンク版関係なく Google アナリティクスのコードを埋め込んでしまう?

リリース物を自動で作れるかと、最新リリースのzipへのリンクを取得できるかなど (バージョン指定ではなくlatestみたいなの)、一旦調べないとですね。

実は cpprefjp_relative を既定にして CI で build する様にすることの利点がここにありまして、この場合は

https://github.com/cpprefjp/cpprefjp.github.io/archive/refs/heads/master.zip

からローカルでも閲覧できる Zip アーカイブがそのままダウンロードできます。常に最新です (実は今までもこのリンクからダウンロードはできたのですが中身が絶対リンクなのでちゃんと表示できないのでした)。ファイル名はURL的には master.zip ですが GitHub 側で保存時のファイル名は cpprefjp.github.io-master.zip と指定している様です。

cpperfjp_relative をデフォルトにする方法は二種類ありまして、

  • 方法1: 既定の設定ファイル settings/cpprefjp.py を書き換える: 実は単に
USE_RELATIVE_LINK = True

とするだけです。

  • 方法2: CI の設定を変える CI から呼び出す以下の部分を書き換える

https://github.com/cpprefjp/site/blob/0cebcd7b2f44fb9b5f221a8c9f161c018f826be1/.github/workflows/script/build.sh#L31-L36

マージ手順

ご存知とは思いますが依存関係がありますのでマージ手順は注意が必要です。何となく以下のような感じの手順になるのではないかと思っていますが、実はよく分かっていないので適宜調節していただければと思います。

  1. crsearch マージ
  2. crsearch の新しい version を npm publish
  3. kunai の crsearch に対する依存を更新 (例 cpprefjp/kunai@33e4234), kunai マージ
  4. markdown_to_html, cpprefjp/image, boostjp/image マージ
  5. site_generator マージ
  6. cpprefjp/site, boostjp/site マージ & フル更新 trigger

@faithandbrave
Copy link
Member

Googleアナリティクスだけが問題なら、埋め込んでしまいましょう。
問題が起きたらまた考えるということで。

@akinomyoga
Copy link
Member Author

akinomyoga commented May 31, 2024

ありがとうございます。ではそのようにしましょう。この PR の中でそれも処理してしまいますか。その場合には上記の 方法1方法2 のどちらが良いでしょうか。

@faithandbrave
Copy link
Member

方法1: 既定の設定ファイル settings/cpprefjp.py を書き換える

でよいかと思います。
一旦マージしてから、別PRで作業でもだいじょうぶです。

@akinomyoga
Copy link
Member Author

ありがとうございます! では今晩 settings/cpprefjp.py を編集してここに push します。一応別コミットにしますね。

@akinomyoga
Copy link
Member Author

69881c0 既定で相対リンクを使うようにしました。また、これに伴って新しいファイル settings/{cpprefjp,boostjp}_relative.py は追加せずに、既存の settings/{cpprefjp,boostjp}.py を書き換える形になりました。

@faithandbrave
Copy link
Member

対応ありがとうございます!
これで問題なさそうであればマージ作業をしていきますので、指摘や相談ごとなどあれば2〜3日中くらいにお願いしますー

@akinomyoga
Copy link
Member Author

取り敢えず問題ありません! よろしくおねがいいたします!

@yumetodo
Copy link
Member

yumetodo commented Jun 2, 2024

settings/{cpprefjp,boostjp}_relative.py は追加せずに、既存の settings/{cpprefjp,boostjp}.py を書き換える形になりました。

これを見落として2時間位ModuleNotFoundError: No module named 'settings.cpprefjp_relative'と戦う凡ミスをしましたorz

ようやく理解したのでビルド開始・・・

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

3 participants