multi-repo support #14

fleksin · 2019-07-15T06:29:06Z

Usage

As title suggested, this PR is aimed to add new capability to extract and display multiple doc repos at the same time.

To Achieve that, I added new options for multiple repos supports: multiRepoUrls
example:

plugins: [
  ...
  [
    'vuepress-plugin-yuque',
   /**
    * new option: multiRepoUrls
    * @property {string} repoRoute - route for the individual repo, must given. doesn't support '/'
    * @property {string} repoUrl - the url of the repo
    */
    multiRepoUrls: [
    {
       repoRoute: 'a',
       repoUrl: 'https://www.yuque.com/zhoufan-jaod3/kb',
     },
     {
       repoRoute: 'b',
       repoUrl: 'https://www.yuque.com/zhoufan-jaod3/cd1bqa',
     },
     ],
  ]
  ....
]

So for each repo, you have to assign a unique repoRoute, which will be added into router.

Design

So one of the obstacles is to resolve multiple sidebar.
Luckily vuepress supports multiple sidebar:
https://v1.vuepress.vuejs.org/theme/default-theme-config.html#multiple-sidebars
which is why the new option requires repoRoute as an input.

After that there is not much trouble implementing multiple repos.

Changes

There are couple of things that I changed during the course.

rewrote `getSideBarByToc` function at `lib/getSideBar.js`

I basically rewrote this function because the old one has bug and it's hard to fix the bug within the old code.
The old function has a problem recognizing a node's position in the tree. If the depth of previous node is greater than current node by more than 1, its position will be messed up.
take this doc's sidebar for example: https://www.yuque.com/zhoufan-jaod3/cd1bqa/zwcaqw
the original sidebar from yuque should be like this:

the pic below shows the generated sidebar of the old and the new function side by side

the left one is using the old function which is incorrect
the right one is the result of the new function

I'm considering adding more unit tests for the new function.

parse the body as `markdown` when format is `lake`

I came across some docs where the format is lake, and it would parse data.body_html instead of data.body. In this case the build command will emit compile error and fail , so I have to treat lake format as markdown to get around it. I don't know the initial motive of this design, so I think I need some opinion on whether this change would break anything.
portal to that line: https://github.com/ulivz/vuepress-plugin-yuque/pull/14/files#r303388389

fleksin · 2019-07-15T11:23:16Z

lib/index.js

+                : typeof source === 'function'
+                ? source(page) === 'markdown'
+                : false
+            if (useMarkdown || data.format === 'markdown' || data.format === 'lake') {


I need some opinion on whether this change would break anything.

If you parse the "lake" as markdown by default, you'll lose some of the styles which is only available in body_html, e.g. width of images.

ulivz

Thanks, please address the comments and write unit tests for the new getSidebar function.

ulivz · 2019-07-21T11:26:47Z

lib/index.js

-    enhanceAppFiles: [
-      path.join(__dirname, 'client.js')
-    ],
+    enhanceAppFiles: [path.join(__dirname, 'client.js')],


Do not format unrelated codes.

ulivz · 2019-07-21T11:26:59Z

lib/index.js

-    plugins: [
-      ['@vuepress/medium-zoom', true],
-    ],
+    plugins: [['@vuepress/medium-zoom', true]],


ulivz · 2019-07-21T11:27:10Z

lib/index.js

-      config
-        .options
-        .html(false)
+      config.options.html(false)


ulivz · 2019-07-21T11:27:34Z

lib/index.js


      if (repoId) {
        assert(
-          typeof repoId === 'string' ||
-          typeof repoId === 'number',
+          typeof repoId === 'string' || typeof repoId === 'number',


ulivz · 2019-07-21T11:28:34Z

lib/index.js

          `[${PACKAGE_NAME}] repoId should be string or number, but got ${repoId}`
        )
      } else if (repoUrl) {
        assert(
          typeof repoUrl === 'string',
          `[${PACKAGE_NAME}] repoUrl should be string, but got ${repoId}`
        )
+      } else if (multiRepoUrls) {
+        assert(Array.isArray(multiRepoUrls), 'multiRepoUrls must be array')


multiRepoUrls must be array => expect "multiRepoUrls" to be an array

ulivz · 2019-07-21T11:29:17Z

lib/index.js

+      } else if (multiRepoUrls) {
+        assert(Array.isArray(multiRepoUrls), 'multiRepoUrls must be array')
+        multiRepoUrls.forEach(repo => {
+          assert(typeof repo === 'object', `[${PACKAGE_NAME}] multiRepoUrls must be object`)


expect each item of "multiRepoUrls" to be a plain object

ulivz · 2019-07-21T11:31:06Z

lib/index.js

+        assert(Array.isArray(multiRepoUrls), 'multiRepoUrls must be array')
+        multiRepoUrls.forEach(repo => {
+          assert(typeof repo === 'object', `[${PACKAGE_NAME}] multiRepoUrls must be object`)
+          assert('repoRoute' in repo, `[${PACKAGE_NAME}] must have repoRoute for multiRepoUrls`)


expect each item of "multiRepoUrls" to have a field "repoRoute"

ulivz · 2019-07-21T11:35:49Z

lib/index.js

+                : typeof source === 'function'
+                ? source(page) === 'markdown'
+                : false
+            if (useMarkdown || data.format === 'markdown' || data.format === 'lake') {


If you parse the "lake" as markdown by default, you'll lose some of the styles which is only available in body_html, e.g. width of images.

ulivz · 2019-07-21T11:36:04Z

lib/index.js


-        const {


Do not format unrelated codes.

fleksin added 4 commits July 15, 2019 14:27

multi-repo support

c0827bf

remove comment

3bc3c5c

feix check

4c58c0c

keep default empty user

ccafb7e

fleksin commented Jul 15, 2019

View reviewed changes

ulivz suggested changes Jul 21, 2019

View reviewed changes

revert some format

30da7db

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

multi-repo support #14

multi-repo support #14

Uh oh!

fleksin commented Jul 15, 2019 •

edited

Loading

Uh oh!

fleksin Jul 15, 2019 •

edited

Loading

Uh oh!

ulivz Jul 21, 2019

Uh oh!

ulivz left a comment

Uh oh!

ulivz Jul 21, 2019

Uh oh!

ulivz Jul 21, 2019

Uh oh!

ulivz Jul 21, 2019

Uh oh!

ulivz Jul 21, 2019

Uh oh!

ulivz Jul 21, 2019 •

edited

Loading

Uh oh!

ulivz Jul 21, 2019 •

edited

Loading

Uh oh!

ulivz Jul 21, 2019

Uh oh!

ulivz Jul 21, 2019

Uh oh!

ulivz Jul 21, 2019

Uh oh!

Uh oh!

Uh oh!

multi-repo support #14

Are you sure you want to change the base?

multi-repo support #14

Uh oh!

Conversation

fleksin commented Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage

Design

Changes

rewrote getSideBarByToc function at lib/getSideBar.js

parse the body as markdown when format is lake

Uh oh!

fleksin Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulivz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulivz Jul 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulivz Jul 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fleksin commented Jul 15, 2019 •

edited

Loading

rewrote `getSideBarByToc` function at `lib/getSideBar.js`

parse the body as `markdown` when format is `lake`

fleksin Jul 15, 2019 •

edited

Loading

ulivz Jul 21, 2019 •

edited

Loading

ulivz Jul 21, 2019 •

edited

Loading