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 option to show link in meta data for editing posts #278

Merged
merged 8 commits into from
Mar 23, 2021

Conversation

Syphdias
Copy link
Contributor

@Syphdias Syphdias commented Mar 1, 2021

The idea was inspired by the stapelberg theme.

If editPostUrl is set, its value will be used to create a link for every post
is displayed in the meta data of the post (if meta not hidden). This is useful
for providing the reader with an easy way to find the proper place to suggest
changes. An example would be an edit prefix for the proper git repo, e.g.:

Params:
  editPostUrl: "https://github.com/adityatelange/hugo-PaperMod/edit/exampleSite"

For the post saved in content/posts/markdown-syntax.md, this will lead to an
href of
"https://github.com/adityatelange/hugo-PaperMod/edit/exampleSite/content/posts/markdown-syntax.md"

The text for the edit button defaults to "Edit" or a translation if available
but can be changed individually by setting the parameter editPostText.

This closes #238.

PS: There is one issue that is not being addressed by this PR and that is docs – and I don't think I can directly solve this in this PR.
Where is documentations supposed to go?

  • README.md on branch master?
  • Wiki of this repo?
  • content/posts/papermod/papermod-variables.md on branch exampleSite
  • Multiple places? (<-wrong answer)

Also:

  • Before changes are merged?
  • After changes are merged?
  • At the same time?

The idea was inspired by the [stapelberg theme].

If `editPostUrl` is set, its value will be used to create a link for every post
is displayed in the meta data of the post (if meta not hidden). This is useful
for providing the reader with an easy way to find the proper place to suggest
changes. An example would be an edit prefix for the proper git repo, e.g.:
```
Params:
  editPostUrl: "https://github.com/adityatelange/hugo-PaperMod/edit/exampleSite"
```
For the post saved in `content/posts/markdown-syntax.md`, this will lead to an
href of
"https://github.com/adityatelange/hugo-PaperMod/edit/exampleSite/content/posts/markdown-syntax.md"

The text for the edit button defaults to "Edit" or a translation if available
but can be changed individually by setting the parameter `editPostText`.

This closes adityatelange#238.

[stapelberg theme]: https://github.com/stapelberg/hugo/blob/d0b2a3edb61a855d87debc3d974ed250853aa931/themes/stapelberg/layouts/_default/single.html#L46
Copy link
Owner

@adityatelange adityatelange left a comment

Choose a reason for hiding this comment

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

  1. How about adding this to post_meta partial so that it would not display extra · when other meta params are empty
  2. It would be appropriate to keep editPost params with each other
    like shown below in site config
    params:
        editPost:
            URL: https://github.com/adityatelange/hugo-PaperMod/tree/exampleSite
            Text: "Edit"

- Instead of two separate parameters `editPostUrl` and `editPostText` there is
  now the `editPost` hash with `URL` and `Text`
```
Params:
  editPost:
    URL: "https://github.com/adityatelange/hugo-PaperMod/edit/exampleSite"
    Text: "Suggest Changes"
```
- Moved insertion of link to post_meta to prevent extra `·` from showing up
@Syphdias
Copy link
Contributor Author

Syphdias commented Mar 1, 2021

  1. How about adding this to post_meta partial so that it would not display extra · when other meta params are empty

I moved the link to post_meta but I am unsure how to deal with slice. When I added (slice (...)) around the printf I got stuck with an extra · separator but no text after it. I assume there was an extra element added to the meta list but an empty one.
To check if I made a mistake elsewhere I tried to simplify it:

{{ $scratch.Add "meta" (slice "foo") }} # didn't work
{{ $scratch.Add "meta" "bar" }}         # worked

I have no idea why slice is needed in the above cases but my case doen't seem to need it(?). Let me know if I did something wrong here.

  1. It would be appropriate to keep editPost params with each other

Makes sense. Changed.

@adityatelange
Copy link
Owner

@Syphdias I see edit button in list pages, we should find a way to hide this
image

@Syphdias
Copy link
Contributor Author

Syphdias commented Mar 1, 2021

Hm, interesting, same for archives since it uses the post_meta partial as well.

I suggest going back to a separate partial like the translation again. This way it can easily be assured to only be used where appropriate, i.e. single.

I tired it analogues to the translations:
image
or
image

Also I found this dandy line which I could also use to only display the separator if there is date, auther, etc.:

{{- if or .Params.author $.Site.Params.author (.Param "ShowReadingTime") (not .Date.IsZero) }}&nbsp;|&nbsp;{{- end -}}

So a few questions:

  • Which order do you prefer? meta, edit, translation or meta, translation, edit?
  • Which separator? | or ·?
    Also possible: meta1 · meta2 · edit | translation or
    meta1 · meta2 | edit · translation / meta1 · meta2 | translation · edit or
    meta1 · meta2 · edit · translation / meta1 · meta2 · translation · edit

@mohnoor94
Copy link
Contributor

+1 nice feature!

The edit link is not wanted in search archives and list view. This is why the
partial edit_post was created like previously. Some adjustments to the
translation_list partial were necessary to show separator if not meta is
available but edit link is shown.
@Syphdias
Copy link
Contributor Author

Syphdias commented Mar 6, 2021

@Syphdias I implemented one of my suggestions from my last comment. I decided to go with meta1 · meta2 | edit | translation.
The reasoning behind it: Using · to softly separate meta data of post (date, reading time, author(s)) and | to strongly separate meta data from edit link as well as translation list.

I also picked up @kdkasad's suggestion to make the URL more flexible.

Would be great if you could give it another look.

@Syphdias
Copy link
Contributor Author

Hey @adityatelange, I was wondering if you could give it another look 🙂

@adityatelange
Copy link
Owner

Hey @adityatelange, I was wondering if you could give it another look

Yes

Copy link
Owner

@adityatelange adityatelange left a comment

Choose a reason for hiding this comment

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

Everything looks good to me :) , just can we move Edit link to end of post-meta ? Also how about opening links in new tab (don't forget to add rel="noopener noreferrer")?


Also maybe we should add a param named NoGit so that @kdkasad's previous comment could be taken into consideration so that a mailto link could be added for non-git edit requests

As if NonGit = true we could only link URL and not the path with it


Also I have doubt with usage of .Param with string values, empty strings can make issues somehow..
refer: https://gohugo.io/functions/param/ and gohugoio/hugo#3366

in that case {{ .Params.editPost.URL | default .Site.Params.editPost.URL }} should be use imo.

- swap translation_list and edit_post
- avoid `.Param`
- add `rel="noopener noreferrer"`
- add `target="_blank"`
@Syphdias
Copy link
Contributor Author

Everything looks good to me :) , just can we move Edit link to end of post-meta ? Also how about opening links in new tab (don't forget to add rel="noopener noreferrer")?

This is three things:

  • "end of post-meta": The link is already after the post-meta So I think you mean "after the translation"? Like this? meta1 · meta2 | translation | edit => need feedback if interpretation was correct
  • rel="noopener noreferrer": Frankly, I am not quite sure what these mean but it sounds SEO related and seems to make sense here. So I added it. => done
  • Opening in new tap. The rel options don't do that, so you probably mean target="_blank" unless I am missing something. I am a big hater of the option because it gets overused and I use middle mouse button or ctrl, if I want to open in a new tab (I even use Death To _blank) but here I think it makes sense. Also added. => feedback needed if this was wrong.

How wrong did I interpret this?

Also maybe we should add a param named NoGit so that @kdkasad's previous comment could be taken into consideration so that a mailto link could be added for non-git edit requests

As if NonGit = true we could only link URL and not the path with it

mailto:// already works:
With URL: "mailto://mail@example.com?subject=Suggesting changes for " you get:

<a href="mailto://mail@example.com?subject=Suggesting%20changes%20for%20/posts/my-first-post.md" rel="noopener noreferrer" target="_blank">Edit</a>

So I would suggest adding another option, if you do not want the page. Something like editPost.appendFilePath: false. I'm also open to defaulting to false and having to manually set it to true if you want to do something like the initial idea.
=> needs feedback about idea, variable name, and default

Also I have doubt with usage of .Param with string values, empty strings can make issues somehow..
refer: https://gohugo.io/functions/param/ and gohugoio/hugo#3366

in that case {{ .Params.editPost.URL | default .Site.Params.editPost.URL }} should be use imo.

=> done, I hope I got everything. Question though: There is $.Site.Params.author – should I use the $ prefix for $.Site.Params.editPost.URL?

@Syphdias Syphdias requested a review from adityatelange March 22, 2021 21:23
@adityatelange
Copy link
Owner

This is three things:

  • "end of post-meta": The link is already after the post-meta So I think you mean "after the translation"? Like this? meta1 · meta2 | translation | edit => need feedback if interpretation was correct

End of post-meta means at the last position in the class post-meta

  • rel="noopener noreferrer": Frankly, I am not quite sure what these mean but it sounds SEO related and seems to make sense here. So I added it. => done
  • Opening in new tap. The rel options don't do that, so you probably mean target="_blank" unless I am missing something. I am a big hater of the option because it gets overused and I use middle mouse button or ctrl, if I want to open in a new tab (I even use Death To _blank) but here I think it makes sense. Also added. => feedback needed if this was wrong.

Opening in new tab target="_blank" is a necessary code so I didn't mention that but the rel one.

How wrong did I interpret this?

No you didn't.

So I would suggest adding another option, if you do not want the page. Something like editPost.appendFilePath: false. I'm also open to defaulting to false and having to manually set it to true if you want to do something like the initial idea.
=> needs feedback about idea, variable name, and default

Okay editPost.appendFilePath is fine, should be defaulting to false as most people will use git providers link

Also I have doubt with usage of .Param with string values, empty strings can make issues somehow..
refer: https://gohugo.io/functions/param/ and gohugoio/hugo#3366
in that case {{ .Params.editPost.URL | default .Site.Params.editPost.URL }} should be use imo.

=> done, I hope I got everything. Question though: There is $.Site.Params.author – should I use the $ prefix for $.Site.Params.editPost.URL?

If it works no need to (info here https://gohugo.io/templates/introduction/#2-use--to-access-the-global-context)

@adityatelange
Copy link
Owner

Where is documentations supposed to go?

  • README.md on branch master?
  • Wiki of this repo?
  • content/posts/papermod/papermod-variables.md on branch exampleSite
  • Multiple places? (<-wrong answer)

Wiki of this repo. I clone the same to exampleSite, and not necessary it is up to date.
Yes it is multiple places
It just made that way so that new users may get an idea of stuff added into without visiting the wiki.

@Syphdias
Copy link
Contributor Author

End of post-meta means at the last position in the class post-meta

post-meta is used in single (where we want the translations and edit), search and archives. If I put the edit button back into post-meta it will show up in two extra places where it is unwanted. I tried to differentiate between but I did not find a way.
I also think it is cleaner and more concise to have a separate partial. Like translation it isn't really meta data but data that ties multiple places together. In case of translation it connects two posts. In the case of edit it connects to a way to edit the article.
Is there any reason you want it in the post-meta file.

Okay editPost.appendFilePath is fine, should be defaulting to false as most people will use git providers link

But wouldn't you want to append the file path if you used a git provider link?

If it works no need to (info here https://gohugo.io/templates/introduction/#2-use--to-access-the-global-context)

Ah, thank you!

Wiki of this repo. I clone the same to exampleSite, and not necessary it is up to date.

Ah, so the wiki is the complete one and exampleSite is a snapshot in time of the wiki.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Syphdias
Copy link
Contributor Author

Syphdias commented Mar 23, 2021

Okay editPost.appendFilePath is fine, should be defaulting to false as most people will use git providers link

But wouldn't you want to append the file path if you used a git provider link?

I implemented editPost.appendFilePath. Setting editPost.appendFilePath: true is now needed if you want to use the file path as you suggested.

@adityatelange adityatelange merged commit abfdb54 into adityatelange:master Mar 23, 2021
@adityatelange
Copy link
Owner

@Syphdias Nice Work ! 👍 Sorry for keeping this PR for too long.
Also I'll update the docs later on.

@Syphdias Syphdias deleted the edit-post-link branch March 23, 2021 19:16
@Syphdias
Copy link
Contributor Author

Also I'll update the docs later on.

I couldn't edit the wiki directly, so I cloned the .wiki repo and pushed it to separate repo: https://github.com/Syphdias/hugo-PaperMod-wiki

Feel free to change what I have written. Maybe it saves you some time.

@prishs
Copy link

prishs commented Mar 25, 2021

appendFilePath appends the file name with backword slash.
So link becomes https://github.com/....../content/posts\readme.md
Am I missing something? I am using vscode on windows.

@Syphdias
Copy link
Contributor Author

Syphdias commented Mar 26, 2021

Oh, sorry about that.

Am I missing something? I am using vscode on windows.

VSCode has nothing to do with it. Apparently .File.Path is dependent on the operating system which is not properly documented and I did not know – it is for .File.Path: https://gohugo.io/variables/files/

Quick and dirty fix below. Warning: Breaks other stuff like \ in a filename.

diff --git a/layouts/partials/edit_post.html b/layouts/partials/edit_post.html
index 21a33eb5..925b2f8c 100644
--- a/layouts/partials/edit_post.html
+++ b/layouts/partials/edit_post.html
@@ -1,6 +1,7 @@
+{{- $fileUrlPath := path.Join (split .File.Path "\\") }}
 {{- if or .Params.editPost.URL .Site.Params.editPost.URL -}}
 {{- if or .Params.author $.Site.Params.author (.Param "ShowReadingTime") (not .Date.IsZero) .IsTranslated }}|&nbsp;{{- end -}}
-<a href="{{ .Params.editPost.URL | default .Site.Params.editPost.URL }}{{ if .Params.editPost.appendFilePath | default ( .Site.Params.editPost.appendFilePath | default false ) }}/{{ .File.Path }}{{ end }}" rel="noopener noreferrer" target="_blank">
+<a href="{{ .Params.editPost.URL | default .Site.Params.editPost.URL }}{{ if .Params.editPost.appendFilePath | default ( .Site.Params.editPost.appendFilePath | default false ) }}/{{ $fileUrlPath }}{{ end }}" rel="noopener noreferrer" target="_blank">
     {{- .Params.editPost.Text | default (.Site.Params.editPost.Text | default (i18n "edit_post" | default "Edit") ) -}}
 </a>
 {{- end }}

I guess the right way would be to use path.Split (in a loop) and the join with /. Also there is a weird note for path.Join:

Note: All path elements on Windows are converted to slash ('/') separators.

Does that mean a this would convert a path? {{- $fileUrlPath := path.Join .File.Path }} to unix style? Could you give that a try, @openact? I don't have a Winowds easily accessible right now.

@prishs
Copy link

prishs commented Mar 26, 2021

yes.. i checked this.. @Syphdias .. this fix is working in windows..

@Syphdias
Copy link
Contributor Author

@openact The patch or the path.Join .File.Path?

@prishs
Copy link

prishs commented Mar 27, 2021

I tried patch.. I did not get what to do in path.join .File.Path.. can you elaborate more?
I do not have much knowledge in this..

@Syphdias
Copy link
Contributor Author

Syphdias commented Mar 27, 2021

Let's say you have these file:

content/posts
└── bla foo
    ├── baz\qux.md
    └── quux.md
  • Linux: bla foo/baz\qux.md
    Windows will probably have trouble with the backslash in the file name.
  • Linux: bla foo/quux.md
    Windows: bla foo\quux.md

The patch does something simple: Take the .File.Path and split it at \ and the join it again. If there is no \ like in bla foo/quux.md while on Linux, no harm done.
But if you look at bla foo/baz\qux.md, it will get messed up into bla foo/baz/qux.md which is wrong.

.File.Path yields differnet results for WIndows and Linux, but the function path.Join is always Linux style apparently. So my idea was to use this property to convert a windows path into a Linux path. My (probably wrong) hope was that you don't have to split .File.Path first but directly use it with path.Join.
If you want to try it, replace path.Join (split .File.Path "\\") from the patch with path.Join (path.Split .File.Path). I don't expect it to work but would be nice if it did.

If it does not work, the proper way to do this would be to use path.Split (OS aware) multiple times for each directory in .File.Path and the combine them again with path.Join.

Edit: probably path.Join (path.Split .File.Path) not path.Join .File.Path

@Syphdias
Copy link
Contributor Author

Edit: probably path.Join (path.Split .File.Path) not path.Join .File.Path

Nope, I was right the first time...
path.Join .File.Path oddly converts into URL/Linux style paths. I'll open a PR...

I might be wrong, but I think this OS-independent behavior is intentional because it look like hugo uses this function internally to build URLs. So this should not break in future releases. *knock on wood*

PS: Fun fact: I can't normally git checkout my current test repo HEAD since it includes a file with \ in the name. 😂

Syphdias added a commit to Syphdias/hugo-PaperMod that referenced this pull request Mar 28, 2021
`.File.Path` is OS dependant and uses `\` as directory separator. This leads to
cases where posts get the wrong URL, if in a sub folder, e.g.
`content/posts/folder\post.md`.

One idea was to replace `\` with `/` this however breaks valid files (on Unix
systems) which include `\` in the file name.
According to the docs [`path.Join`](layouts/partials/edit_post.html) is OS
unaware and converts to a Unix-like path.

> Note: All path elements on Windows are converted to slash ('/') separators.

PS: It looks like hugo also uses this internally to build URLs.
PPS: Solution was discussed in adityatelange#278.
@Syphdias
Copy link
Contributor Author

@openact, checkout #333 if it fixes your problem.

adityatelange pushed a commit that referenced this pull request Mar 28, 2021
`.File.Path` is OS dependant and uses `\` as directory separator. This leads to
cases where posts get the wrong URL, if in a sub folder, e.g.
`content/posts/folder\post.md`.

One idea was to replace `\` with `/` this however breaks valid files (on Unix
systems) which include `\` in the file name.
According to the docs [`path.Join`](layouts/partials/edit_post.html) is OS
unaware and converts to a Unix-like path.

> Note: All path elements on Windows are converted to slash ('/') separators.

PS: It looks like hugo also uses this internally to build URLs.
PPS: Solution was discussed in #278.
kylethedeveloper pushed a commit to kylethedeveloper/hugo-PaperMod that referenced this pull request Feb 21, 2023
…#278)

Usage

- in site config =>

    Params:
    editPost:
        URL: "https://github.com/<path_to_repo>/content"
        Text: "Suggest Changes" # edit text 
        appendFilePath: true # to append file path to Edit link

- in front-matter vars =>
    ---
    editPost:
        URL: "https://github.com/<path_to_repo>/content"
        Text: "Suggest Changes" # edit text 
        appendFilePath: true # to append file path to Edit link
    ---

- Front-matter vars overrides global ones
kylethedeveloper pushed a commit to kylethedeveloper/hugo-PaperMod that referenced this pull request Feb 21, 2023
`.File.Path` is OS dependant and uses `\` as directory separator. This leads to
cases where posts get the wrong URL, if in a sub folder, e.g.
`content/posts/folder\post.md`.

One idea was to replace `\` with `/` this however breaks valid files (on Unix
systems) which include `\` in the file name.
According to the docs [`path.Join`](layouts/partials/edit_post.html) is OS
unaware and converts to a Unix-like path.

> Note: All path elements on Windows are converted to slash ('/') separators.

PS: It looks like hugo also uses this internally to build URLs.
PPS: Solution was discussed in adityatelange#278.
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.

[Feature] Edit button for posts
5 participants