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

Add PreviewURL field #104

Merged
merged 3 commits into from
Oct 21, 2023
Merged

Add PreviewURL field #104

merged 3 commits into from
Oct 21, 2023

Conversation

Mahito
Copy link
Contributor

@Mahito Mahito commented Oct 20, 2023

entryHeader に PreviewURL のフィールドを追加

related: #103

entryHeader に以下のフィールドを追加
- PreviewURL
- IsDraft
@Songmu
Copy link
Member

Songmu commented Oct 20, 2023

早速ありがとうございます!
IsPreviewはフィールドとしてもFrontMatterでもなくてよいかと思いました。(Preview URLの有無で自明なので)

とりあえずは、「Draft時には自動的にPreviewURLを発行する」というシンプルな仕様で良いかと思います。

もし、PreviewURLを発行したくないという要望が来た時点でどのようにするかを考えるのが良いかと思っていますし、個人的にはあまり対応の必要性は感じていません。

また、仮に将来的に対応するとしても、entryHeader に情報を増やすよりかは、コマンドラインオプションで --no-preview などを追加するほうが良いかなぁとも思っています。

なので、とりあえずは IsPreview を削ってもらいたいです。

@Mahito Mahito changed the title Add preview option Add PreviewURL field Oct 20, 2023
@coveralls
Copy link

coveralls commented Oct 20, 2023

Pull Request Test Coverage Report for Build 6597107554

  • 0 of 14 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.3%) to 30.329%

Changes Missing Coverage Covered Lines Changed/Added Lines %
entry.go 0 14 0.0%
Totals Coverage Status
Change from base Build 6518989774: -0.3%
Covered Lines: 212
Relevant Lines: 699

💛 - Coveralls

entry.go Outdated
IsDraft: isDraft,
URL: &entryURL{u},
EditURL: editLink.Href,
PreviewURL: previewLink.Href,
Copy link
Member

Choose a reason for hiding this comment

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

PreviewLinkはnilableだと思うので、nilチェックするようにして下さい。
(調べてはいませんが、公開後のエントリーや、はてなブログ管理画面から作られた下書きでpreviewを発行していないものは link rel="preview" がない気がしますし、その可能性はケアしておいたほうが良いと思います

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

実装時に PreviewLink が nil のケースは考えたのですが、コメントにあるように公開後や preview=no の場合 PreviewLink に nil が来るケースはあると思います。
しかし、alternateLink や EditLink と同様に nil が来た際エラーとして return するべきかというと、PreviewLink の場合 nil のケースは上記のように正常動作として許容されるべきかと思います。結果、nil チエックをしたとして、そのチェックの結果何を処理すべきかが思いつかずとりあえずひとまず nil チェックを飛ばした次第です。

PreviewLink が nil の場合の処理どうしましょうか・・・?

Copy link
Member

Choose a reason for hiding this comment

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

確認ありがとうございます。previewLinkがnilの場合、previewLink.Hrefにアクセスするとnil pointer exceptionが起きてしまうので、ハンドリングは必要です。
entryHeader.PreviewURL をomitemptyに設定してもらっているので、プレビューリンクがない場合に空文字列を指定するという方針で良いのではないでしょうか。空文字列を渡しておけばプFrontMatterに書かれなくなるので。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

previewLink.Href のこと忘れてましたので nil チェック追加しました 🙇

@Songmu Songmu added the minor label Oct 20, 2023
@Songmu
Copy link
Member

Songmu commented Oct 21, 2023

Thank you!

@Songmu Songmu merged commit fea1afb into x-motemen:master Oct 21, 2023
7 checks passed
@github-actions github-actions bot mentioned this pull request Oct 21, 2023
@Mahito Mahito deleted the add_preview_option branch October 23, 2023 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants