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

More powerful repository config #57

Open
salim-b opened this issue Sep 18, 2023 · 10 comments
Open

More powerful repository config #57

salim-b opened this issue Sep 18, 2023 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@salim-b
Copy link
Contributor

salim-b commented Sep 18, 2023

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.

@h-enk h-enk added the enhancement New feature or request label Oct 27, 2023
@h-enk h-enk self-assigned this Jan 8, 2024
@james-d-elliott
Copy link

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
Copy link
Contributor Author

salim-b commented Mar 5, 2024

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 🙂

@james-d-elliott
Copy link

james-d-elliott commented Mar 5, 2024

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
Copy link

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

@salim-b
Copy link
Contributor Author

salim-b commented Mar 5, 2024

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
Copy link

james-d-elliott commented Mar 5, 2024

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>

@h-enk h-enk assigned james-d-elliott and unassigned h-enk Mar 5, 2024
@h-enk h-enk modified the milestone: v1.5 Mar 5, 2024
@salim-b
Copy link
Contributor Author

salim-b commented Mar 6, 2024

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
Copy link

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
Copy link
Contributor Author

salim-b commented Mar 6, 2024

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:

  • Your snippet above fails to assemble a valid URL if docsreposubpath is unset or "".
  • Using urls.JoinPath to assemble the URLs would be more robust since users could set docsRepo with or without a trailing slash and it'd work in both cases.

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
Copy link

james-d-elliott commented Mar 6, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants