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

Support sidebarAbsolutePath to fix relative link and nested sidebar when I just provide one sidebar.md #1076

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,14 @@ window.$docsify = {
};
```

## sidebarAbsolutePath

In some case, you have only one sidebar.md in your project. But you may put doc files in different directories. You can set `sidebarAbsolutePath` to be `true`, so that it will automaticly make the given `loadSidebar` to be the only one to point at.

It should works with `loadSidebar`. If you do not set `loadSidebar`, don't set this option any more.

It works well with `relativePath` and `basePath`. You do not need to worry about the path conflict.

Copy link
Member

Choose a reason for hiding this comment

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

These docs don't describe what the option actually does. It doesn't mention that it can be a string. Can you please update?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @trusktr. There also seems to be some gramma thats not correct

## coverpage

- Type: `Boolean|String|String[]|Object`
Expand Down Expand Up @@ -587,4 +595,4 @@ Adds a space on top when scrolling content page to reach the selected section. T
window.$docsify = {
topMargin: 90, // default: 0
};
```
```
4 changes: 2 additions & 2 deletions packages/docsify-server-renderer/index.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { readFileSync } from 'fs';
import { resolve, basename } from 'path';
import resolvePathname from 'resolve-pathname';
import fetch from 'node-fetch';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be reverted to the previous state. There is no point in rearranging imports if this is the only changes in the file

Copy link
Member

Choose a reason for hiding this comment

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

This is a result of the automated husky lint process, which modifies the files. The git commit step should not leave modifications in the working tree.

import debug from 'debug';
import { AbstractHistory } from '../../src/core/router/history/abstract';
import { Compiler } from '../../src/core/render/compiler';
import { isAbsolutePath } from '../../src/core/router/util';
import * as tpl from '../../src/core/render/tpl';
import { prerenderEmbed } from '../../src/core/render/embed';
import fetch from 'node-fetch';
import debug from 'debug';

function cwd(...args) {
return resolve(process.cwd(), ...args);
Expand Down
2 changes: 1 addition & 1 deletion src/core/event/scroll.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Tweezer from 'tweezer.js';
import { isMobile } from '../util/env';
import * as dom from '../util/dom';
import config from '../config';
import Tweezer from 'tweezer.js';

const nav = {};
let hoverOver = false;
Expand Down
4 changes: 2 additions & 2 deletions src/core/global-api.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import prism from 'prismjs';
import marked from 'marked';
import * as util from './util';
import * as dom from './util/dom';
import { Compiler } from './render/compiler';
import { slugify } from './render/slugify';
import { get } from './fetch/ajax';
import prism from 'prismjs';
import marked from 'marked';

export default function() {
window.Docsify = {
Expand Down
31 changes: 27 additions & 4 deletions src/core/render/compiler.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import marked from 'marked';
import { isAbsolutePath, getPath, getParentPath } from '../router/util';
import { isFn, merge, cached, isPrimitive } from '../util/core';
import { tree as treeTpl } from './tpl';
Expand All @@ -11,7 +12,6 @@ import { paragraphCompiler } from './compiler/paragraph';
import { taskListCompiler } from './compiler/taskList';
import { taskListItemCompiler } from './compiler/taskListItem';
import { linkCompiler } from './compiler/link';
import marked from 'marked';

const cachedLinks = {};

Expand Down Expand Up @@ -70,9 +70,12 @@ export class Compiler {
this.contentBase = router.getBasePath();

const renderer = this._initRenderer();
const mdConf = config.markdown || {};

this.renderer = renderer;
this.heading = renderer.heading;

let compile;
const mdConf = config.markdown || {};

if (isFn(mdConf)) {
compile = mdConf(marked, renderer);
Expand Down Expand Up @@ -218,7 +221,7 @@ export class Compiler {
renderer,
router,
linkTarget,
compilerClass: _self,
compiler: _self,
});
origin.paragraph = paragraphCompiler({ renderer });
origin.image = imageCompiler({ renderer, contentBase, router });
Expand All @@ -230,6 +233,26 @@ export class Compiler {
return renderer;
}

compileSidebar(text) {
if (!this.config.sidebarAbsolutePath) {
return this.compile(text);
}

const { renderer, linkTarget, router } = this;

linkCompiler({
renderer,
router,
linkTarget,
compiler: this,
isSidebar: true,
});

const html = this.compile(text);
this.renderer.link = this.renderer.origin.link;
return html;
}

/**
* Compile sidebar
* @param {String} text Text content
Expand All @@ -242,7 +265,7 @@ export class Compiler {
let html = '';

if (text) {
html = this.compile(text);
html = this.compileSidebar(text);
} else {
for (let i = 0; i < toc.length; i++) {
if (toc[i].ignoreSubHeading) {
Expand Down
29 changes: 23 additions & 6 deletions src/core/render/compiler/link.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,37 @@
import { getAndRemoveConfig } from '../utils';
import { isAbsolutePath } from '../../router/util';

export const linkCompiler = ({ renderer, router, linkTarget, compilerClass }) =>
(renderer.link = (href, title = '', text) => {
export const linkCompiler = ({
renderer,
router,
linkTarget,
compiler,
isSidebar = false,
}) => {
const link = (href, title = '', text) => {
let attrs = [];
const { str, config } = getAndRemoveConfig(title);

title = str;

if (
!isAbsolutePath(href) &&
!compilerClass._matchNotCompileLink(href) &&
!compiler._matchNotCompileLink(href) &&
!config.ignore
) {
if (href === compilerClass.config.homepage) {
const sidebarAbsolutePath = compiler.config.sidebarAbsolutePath;
const basePath =
Copy link
Contributor

Choose a reason for hiding this comment

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

To me the following lines has a weird formatting.

I would suggest doing this instead:

Suggested change
const basePath =
const basePath = typeof sidebarAbsolutePath === 'string'
? sidebarAbsolutePath
: router.getBasePath();
const relativeTo = isSidebar && sidebarAbsolutePath
? basePath
: router.getCurrentPath();

typeof sidebarAbsolutePath === 'string'
? sidebarAbsolutePath
: router.getBasePath();
const relativeTo =
isSidebar && sidebarAbsolutePath ? basePath : router.getCurrentPath();

if (href === compiler.config.homepage) {
href = 'README';
}

href = router.toURL(href, null, router.getCurrentPath());
href = router.toURL(href, null, relativeTo);
} else {
if (!isAbsolutePath(href) && href.startsWith('./')) {
href =
Expand Down Expand Up @@ -48,4 +62,7 @@ export const linkCompiler = ({ renderer, router, linkTarget, compilerClass }) =>
}

return `<a href="${href}" ${attrs.join(' ')}>${text}</a>`;
});
};
renderer.link = link;
return link;
};
2 changes: 1 addition & 1 deletion src/core/render/embed.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import stripIndent from 'strip-indent';
import { get } from '../fetch/ajax';
import { merge } from '../util/core';
import stripIndent from 'strip-indent';

const cached = {};

Expand Down
2 changes: 1 addition & 1 deletion src/core/render/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable no-unused-vars */
import tinydate from 'tinydate';
import * as dom from '../util/dom';
import cssVars from '../util/polyfill/css-vars';
import { callHook } from '../init/lifecycle';
Expand All @@ -10,7 +11,6 @@ import { scrollActiveSidebar } from '../event/scroll';
import { Compiler } from './compiler';
import * as tpl from './tpl';
import { prerenderEmbed } from './embed';
import tinydate from 'tinydate';

function executeScript() {
const script = dom
Expand Down
14 changes: 13 additions & 1 deletion src/core/router/history/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,20 @@ export class History {
const { config } = this;
const base = this.getBasePath();
const ext = typeof config.ext === 'string' ? config.ext : '.md';
const alias = config.alias || {};
const rules = Object.assign({}, alias);

// auto alias sidebar which is set to be absolute path
if (config.sidebarAbsolutePath) {
const sidebar = config.loadSidebar || '_sidebar.md';
Copy link
Member

@trusktr trusktr Apr 3, 2020

Choose a reason for hiding this comment

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

Do we need this sidebarAbsolutePath option? Or can we just provide an absolute path in loadSidebar? We can use a url tool to detect absolute path, like /path/to/sidebar.md, //path/to/sidebar.md, https://example.com/path/to/sidebar.md, etc.

Copy link
Member

Choose a reason for hiding this comment

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

Think about this: what happend if they set sidebarAbsolutePath to true, but it isn't an absolute path? Should it be an error? Will they be confused?

Here's an idea on how to detect absolute paths with a regex: https://stackoverflow.com/a/19709846/454780 (but we should allow paths starting with / to return true if we want.

Copy link
Member

Choose a reason for hiding this comment

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

Here's an idea on how to detect absolute paths with a regex: https://stackoverflow.com/a/19709846/454780 (but we should allow paths starting with / to return true if we want.

Yes, we should consider detecting too.

const base =
typeof config.sidebarAbsolutePath === 'string'
? config.sidebarAbsolutePath
: this.getBasePath();
rules['/.*/' + sidebar] = base + sidebar;
}

path = config.alias ? getAlias(path, config.alias) : path;
path = getAlias(path, rules);
path = getFileName(path, ext);
path = path === `/README${ext}` ? config.homepage || path : path;
path = isAbsolutePath(path) ? path : getPath(base, path);
Expand Down
2 changes: 1 addition & 1 deletion test/unit/base.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
require = require('esm')(
module /* , options */
); /* eslint-disable-line no-global-assign */
const { History } = require('../../src/core/router/history/base');
const { expect } = require('chai');
const { History } = require('../../src/core/router/history/base');

class MockHistory extends History {
parse(path) {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/render.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { init, expectSameDom } = require('../_helper');
const { expect } = require('chai');
const { init, expectSameDom } = require('../_helper');

describe('render', function() {
it('important content (tips)', async function() {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
require = require('esm')(
module /* , options */
); /* eslint-disable-line no-global-assign */
const { resolvePath } = require('../../src/core/router/util');
const { expect } = require('chai');
const { resolvePath } = require('../../src/core/router/util');
trusktr marked this conversation as resolved.
Show resolved Hide resolved

describe('router/util', function() {
it('resolvePath', async function() {
Expand Down