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

feat: add git repo link and edit links #55

Merged
merged 7 commits into from Sep 17, 2020

Conversation

sin-tanaka
Copy link
Contributor

@sin-tanaka sin-tanaka commented Jul 20, 2020

Resolves #52

config file: https://github.com/sin-tanaka/vitepress-playground/blob/master/docs/.vitepress/config.js

スクリーンショット 2020-07-23 20 53 50

with locales

スクリーンショット 2020-07-23 20 54 08

@sin-tanaka sin-tanaka force-pushed the feat/git-repo-link branch 3 times, most recently from bd68705 to 57edeca Compare July 23, 2020 11:51
@sin-tanaka sin-tanaka marked this pull request as ready for review July 23, 2020 11:57
@kiaking
Copy link
Member

kiaking commented Sep 3, 2020

@sin-tanaka Hi! So sorry for the late reply. Thanks for the PR! Could you:

  1. Align configs with current VuePress config? It doesn't need to support all of the items, but it would be nice if we can config it as below.
module.exports = {
  themeConfig: {
    repo: 'vuejs/vuex',
    docsDir: 'docs',
    docsBranch: 'dev',
    editLinkText: 'Edit me!!'
  }
}
  1. Could you rebase the branch to the latest? feat: add external link support for nav items #46 is merged, so it's much easier to read the changes 😅

@sin-tanaka
Copy link
Contributor Author

@kiaking Thanks for the review!

  1. The format of the config is as follows:

https://github.com/vuejs/vitepress/blob/master/src/client/theme-default/config.ts#L7

  export interface Config {
    logo?
    nav?: NavItem[] | false
    sidebar?: SideBarConfig | MultiSideBarConfig
    search?
    editLink?: EditLinkConfig | false
    lastUpdated?
    prevLink?
    nextLink?
  }

Would you prefer to be able to set it to the first level of themeConfig, just like VuePress?

  1. rebased!

@kiaking
Copy link
Member

kiaking commented Sep 4, 2020

@sin-tanaka Thanks for the rebase 🙌

Would you prefer to be able to set it to the first level of themeConfig, just like VuePress?

Yeah I think that would be better. For example having configs as close as VuePress will make migration from VuePress to VitePress a lot easier 👍 Would you mind doing that 😃 ?

@posva
Copy link
Member

posva commented Sep 16, 2020

@kiaking I changed the edit config but then realized it already had a different structure than before. So I'm wondering if I should revert those changes to keep the original new editconfig type

@kiaking
Copy link
Member

kiaking commented Sep 16, 2020

@posva Nice! I think the current config is desired one, since it matches the VuePress config. So I would say we should keep it this way 👍

@@ -43,6 +43,7 @@ export function createMarkdownToVueRenderFn(
title: inferTitle(frontmatter, content),
frontmatter,
headers: data.headers,
relativePath: file.replace(/\\/g, '/'),
Copy link
Member

Choose a reason for hiding this comment

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

@sin-tanaka What does this represent? Why are you replacing / with /? is it for Windows? I saw it in the vuepress code base too

@posva posva changed the title feat: add git repo link feat: add git repo link and edit links Sep 17, 2020
@posva posva merged commit 0ea34cb into vuejs:master Sep 17, 2020
@kiaking kiaking moved this from In progress to Done in v1.0.0-alpha Oct 19, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
v1.0.0-alpha
  
Done
Development

Successfully merging this pull request may close these issues.

Git repository and Edit Links
3 participants