thuliteio / doks-core

Official Doks core integration for the Doks theme
Other
9 stars 13 forks source link

More powerful repository config #57

Open salim-b opened 1 year ago

salim-b commented 1 year ago

Currently, the variable parts for the "Edit this page on ..." and "Last edited on ..." functionality are supposed to be defined via keys repoHost and docsRepo* in site.Params.doks. Although this suggests that Doks would work with a lot of different forges (GitHub, GitLab, Gitea etc. pp.), it actually doesn't. This is because the current template hardcodes an incomplete implementation how forge URLs are constructed. Currently, it is e.g. broken for GitLab.

I suggest to switch to a more elegant approach by allowing to configure the URL construction scheme directly in the config. A full example how this can be done is found in this great post on the Hugo forum. The example only misses the subdirectory part (currently encoded in docsRepoSubPath), besides it should be adaptable pretty-much 1:1 on Doks.

james-d-elliott commented 9 months ago

I'm taking this up, and I'm going to implement the pattern formatting. Can you however describe what the issue with the current URL is and are you using GitLab self-hosted or GitLab cloud?

salim-b commented 9 months ago

Sorry, I must have gotten confused. GitLab works fine with the current template. I guess I mistakenly thought GitLab subgroups like gitlab.com/maingroup/subgroup/project wouldn't work with the current solution, but they actually do.

But there's still an issue with the current template that the one from Joe Mooring does not suffer from (since it uses .File.Filename instead of File.Path): If language-specific content is not stored in lang-subdirs but in files with the language code as suffix like content/index.fr.md, the URLs constructed by Doks current template are wrong. Joe Mooring's solution is always right.

I just tested it with the following three files changed to:

config/_default/params.toml

[repo]
  url = "https://gitlab.com/votelog/websites/votelog.ch"
  host =  "GitLab"
  branch = "main"
  path = "" # path from repo root; only if Hugo project is under subdir
  [repo.pattern]
    commit = '/commit/%s' # var = commit hash
    edit = '/edit/%s/%s' # 1st var  = `branch`, 2nd var = `path`
    view = '/blob/%s/%s' # 1st var  = `branch`, 2nd var = `path`

layouts/partials/main/edit-page.html

{{- with .File }}
  {{- $file := . }}
  {{- with site.Params.repo }}
    {{- $filePathRel := strings.TrimPrefix hugo.WorkingDir $file.Filename }}
    {{- $href := urls.JoinPath .url .path (printf .pattern.edit .branch $filePathRel) }}
    <div class="edit-page">
      <a href="{{ $href }}" rel="external">
        <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-edit-2">
          <path d="M17 3a2.828 2.828 0 1 1 4 4L7.5 20.5 2 22l1.5-5.5L17 3z"></path>
        </svg>
        {{ i18n "edit_page" }} {{ .host }}
      </a>
    </div>
  {{- end }}
{{- end -}}

layouts/partials/main/last-modified.html

{{ if .GitInfo -}}
  {{ with site.Params.repo -}}
    {{- $date := partial "main/date" (dict "Date" $.GitInfo.AuthorDate.Local "Format" $.Site.Params.BookDateFormat) -}}
    <div class="last-modified">
      <a href="{{ urls.JoinPath .url .path (printf .pattern.commit $.GitInfo.Hash)  }}">
        <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-calendar"><rect x="3" y="4" width="18" height="18" rx="2" ry="2"></rect><line x1="16" y1="2" x2="16" y2="6"></line><line x1="8" y1="2" x2="8" y2="6"></line><line x1="3" y1="10" x2="21" y2="10"></line></svg>
        {{ i18n "last_updated" }} {{ $date }}
      </a>
    </div>
  {{ end -}}
{{ end -}}

Feel free to include the above in a PR :slightly_smiling_face:

james-d-elliott commented 9 months ago

Yeah the GitLab links looked fine to me too but I think in general this is a good idea. and appreciate the insight.

I'd ideally prefer to prevent a change which breaks existing users without warning and allow for the backwards compatibility. I also think the subpath compatibility for users hosting the docs in a subpath is useful, admittedly this affects me.

james-d-elliott commented 9 months ago

Also are you saying you confirm this works with the langs in subfolders too?

salim-b commented 9 months ago

Also are you saying you confirm this works with the langs in subfolders too?

Yes.

I'd ideally prefer to prevent a change which breaks existing users without warning and allow for the backwards compatibility.

Sure, I understand. Note that subpaths are still supported with the above (by setting repo.path). The template code could be made more robust for the case when part of the whole repo config is unset, though. Backwards compatibility config-wise I would advise against since that would result in ugly template code.

james-d-elliott commented 9 months ago

Can you give this a try when you get a chance to see there isn't anything obvious missed?

[doks]
  editPage = true # false (default) or true
  lastMod = true # false (default) or true

  ## Edit Link Generation Options. The 'repoHost' option sets a default 'repoURLPattern'.
  repoHost = "GitHub" # GitHub (default), Gitea, GitLab, Bitbucket, or BitbucketServer
  ## The format string override to build the edit URL. 
  ## First parameter is docsRepo, second is docsRepoBranch, and third is the file path (prefixed with docsRepoSubPath if set). 
  ## The format defaults (which this setting overrides) are:
  ##   GitHub: %s/blob/%s/%s
  ##   Gitea: %s/_edit/%s/%s
  ##   GitLab: '%s/~blob/%s/%s'
  ##   Bitbucket: '%s/src/%s/%s'
  ##   BitbucketServer: '%s/browse/%s/%s'
  repoURLPattern = ""
  docsRepo = "https://github.com/authelia/authelia"
  docsRepoBranch = "master" # main (default), master, or <branch name>
  docsRepoSubPath = "" # "" (none, default) or <sub path>
{{ $pattern := site.Params.doks.repoURLPattern | default "" }}
{{ if not $pattern }}
  {{ if (eq site.Params.doks.repoHost "GitHub") }}
    {{ $pattern = "%s/blob/%s/%s" }}
  {{ else if (eq site.Params.doks.repoHost "Gitea") }}
    {{ $pattern = "%s/_edit/%s/%s" }}
  {{ else if (eq site.Params.doks.repoHost "GitLab") }}
    {{ $pattern = "%s/~blob/%s/%s" }}
  {{ else if (eq site.Params.doks.repoHost "Bitbucket") }}
    {{ $pattern = "%s/src/%s/%s" }}
  {{ else if (eq site.Params.doks.repoHost "BitbucketServer") }}
    {{ $pattern = "%s/browse/%s/%s" }}
  {{ end }}
{{ end }}

{{ $path := strings.TrimPrefix "/" (replace (strings.TrimPrefix hugo.WorkingDir .File.Filename) "\\" "/") }}

{{ $url := "" }}

{{ if and (isset .Site.Params.doks "docsreposubpath") (not (eq site.Params.doks.docsRepoSubPath "")) }}
  {{ $url = printf $pattern site.Params.doks.docsRepo site.Params.doks.docsRepoBranch (path.Join site.Params.doks.docsRepoSubPath $path) }}
{{ else }}
  {{ $url = printf $pattern site.Params.doks.docsRepoBranch $path }}
{{ end }}

<div class="edit-page">
  <a href="{{ $url }}">
    <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-edit-2">
      <path d="M17 3a2.828 2.828 0 1 1 4 4L7.5 20.5 2 22l1.5-5.5L17 3z"></path>
    </svg>
    {{ i18n "edit_page" }} {{ site.Params.doks.repoHost }}
  </a>
</div>
salim-b commented 9 months ago

Can you give this a try when you get a chance to see there isn't anything obvious missed?

This is an incomplete implementation since you only allow the user to configure a single pattern to build URLs, while there should be multiple (at least three are relevant for Doks: edit, view and commit).

Also I would prefer the repo config to be hierarchically structured. If you wanna keep it under the doks key, what I mean would be:

config/_default/params.toml

[doks.repo]
  url = "https://gitlab.com/votelog/websites/votelog.ch"
  host =  "GitLab"
  branch = "main"
  path = "" # path from repo root; only if Hugo project is under subdir
  [doks.repo.pattern]
    commit = '/commit/%s' # var = commit hash
    edit = '/edit/%s/%s' # 1st var  = `branch`, 2nd var = `path`
    view = '/blob/%s/%s' # 1st var  = `branch`, 2nd var = `path`

Hardcoding pattern fallbacks in the template is fine (and convenient for users), I guess. 👍

Finally, I wonder whether the path separator normalization for Windows (replace "\\" "/") is actually necessary. Does Hugo really return a different .File.Filename on Windows than on Unix platforms, i.e. doesn't it normalize paths internally already and only return Unix-style path separators?

james-d-elliott commented 9 months ago

This is an incomplete implementation since you only allow the user to configure a single pattern to build URLs, while there should be multiple (at least three are relevant for Doks: edit, view and commit).

Are any of these used currently? If not realistically users can just arbitrarily add their own params.

Also I would prefer the repo config to be hierarchically structured. If you wanna keep it under the doks key, what I mean would be.

Yeah I prefer your structure too. Just avoiding breaking people when it's just nice and not necessary.

Hardcoding pattern fallbacks in the template is fine (and convenient for users), I guess. 👍

Yep, it also fits with the vibe of the existing implementation.

Finally, I wonder whether the path separator normalization for Windows (replace "\" "/") is actually necessary. Does Hugo really return a different .File.Filename on Windows than on Unix platforms, i.e. doesn't it normalize paths internally already and only return Unix-style path separators?

Not entirely sure I didn't initially implement that normalization, but I can check.

salim-b commented 9 months ago

Are any of these used currently?

Yes, see layouts/partials/main/last-modified.html or my example above.

Not entirely sure I didn't initially implement that normalization, but I can check.

Your snippet above includes the normalization. But it would be nice if you could check if it's actually necessary (if you have a Windows machine to test).

Furthermore, two observations:

It would be easier to review if you'd include your changes in your current PR (or submit a new one if you like) :)

Just avoiding breaking people when it's just nice and not necessary.

I think such a breaking change is fine if it's really an improvement (I think it is) and it is mentioned in the release notes :)

james-d-elliott commented 9 months ago

I think such a breaking change is fine if it's really an improvement (I think it is) and it is mentioned in the release notes :)

Yeah that's fine, I'm happy to include that in the discussion I'm not against it - just to me at this point it's a finer detail and functionality is my end-goal. I'll double check everything else you've mentioned then put it in the PR.