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

New pathnameVersion issue-term type #568

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nvuillam
Copy link

@nvuillam nvuillam commented Nov 18, 2021

Fixes #567

New issue-term pathnameVersion, to remove first segment of pathname in case the documentation is versioned

Some documentations are versioned (mkdocs-material with mike, for example )

To not have different comments depending on versions, we need to be able to use my/doc/page as issue-term instead of v5.0.1/my/doc/page

Example code to embed:

<script src="https://utteranc.es/client.js"
        repo="megalinter/megalinter"
        issue-term="pathnameVersion"
        label="comments"
        theme="github-light"
        crossorigin="anonymous"
        async>
</script>

@nvuillam nvuillam mentioned this pull request Nov 18, 2021
Copy link
Contributor

@zsdycs zsdycs left a comment

Choose a reason for hiding this comment

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

I reviewed the code and thought that this modification did meet the requirement of "merging addresses of different versions into the same comment".

However, I have two questions 👇:

  1. Why not do this ("v4.24/my/doc/page".split("/").slice(1).join("/")) before passing in "pathname" with version information.
  2. I want to pass in "pathname" with version information, but when comments of different versions are not merged, "pathname" attrs will still be used.

So I think this ("v4.24/my/doc/page".split("/").slice(1).join("/")) should be processed before setting the "pathname" attrs of "utterances" Processing, not in "utterances".

@nvuillam
Copy link
Author

So you mean that I could build the issue title directly in my website code, then pass it to utterances ?

I could do that, but with the huge number of versioned documentations built with mkdocs, i thought it will be nice to do that within utterances, so it will save time to all the doc maintainers who will use you great tool :)

@zsdycs
Copy link
Contributor

zsdycs commented Nov 19, 2021

So you mean that I could build the issue title directly in my website code, then pass it to utterances ?

I could do that, but with the huge number of versioned documentations built with mkdocs, i thought it will be nice to do that within utterances, so it will save time to all the doc maintainers who will use you great tool :)

😄Whether to merge or not is determined by @jdanyow , but I think the probability of merger is small. "utterances" hasn't added new features for a long time.

@nvuillam
Copy link
Author

@zsdycs i hope ot will be accepted, this will please for sure to all mkdocs / mike users :)

https://github.com/mkdocs/mkdocs

https://github.com/jimporter/mike

@laymonage
Copy link

An alternative I would suggest is adding a regex option to capture whatever part you want from the string to search for the issue. That could work with other mappings as well, such as title, og:title, etc.

Shameless plug: I think that's something I would like to add to my project giscus 👀

@nvuillam
Copy link
Author

@jdanyow any chance to see this PR merged someday ? :)

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: manage comments in a versioned documentation
3 participants