docsify icon indicating copy to clipboard operation
docsify copied to clipboard

feature: support customized sidebar item name from content

Open Jacco-V opened this issue 2 years ago • 14 comments

Add support for setting a customized sidebar item name from the documentation markdown content

See also issue: #1821

Summary

This feature add support for specifying custom sidebar item names (especially for long titles) in the documentation markdown files, so that a the custom (typically shortened) item name is used in the sidebar:

## How would I write an "hello, world" example? :sidebar="hello world?"

image

What kind of change does this PR introduce?

Feature

For any code change,

  • [x] Related documentation has been updated if needed
  • [x] Related tests have been updated or tests have been added

Does this PR introduce a breaking change? (check one)

  • [ ] Yes
  • [x] No

If yes, please describe the impact and migration path for existing applications:

Related issue, if any:

Tested in the following browsers:

  • [x] Chrome
  • [x] Firefox
  • [x] Safari
  • [x] Edge
  • [ ] IE

Jacco-V avatar Jun 22 '22 16:06 Jacco-V

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
docsify-preview ✅ Ready (Inspect) Visit Preview Sep 12, 2022 at 2:15PM (UTC)

vercel[bot] avatar Jun 22 '22 16:06 vercel[bot]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 03fda8b545576617659fccfe78962c930bc94d57:

Sandbox Source
docsify-template Configuration

codesandbox-ci[bot] avatar Jun 22 '22 16:06 codesandbox-ci[bot]

I pushed some changes, but it does not work as expected :( It seems that the :sidebar="hello world?" does not get parsed as config?

## How would I write an "hello, world" example? :sidebar="hello world?"

could it be that the config values read by getAndRemoveConfig do not support quoted strings?

https://github.com/docsifyjs/docsify/blob/1e46f2bd1dbb178ede006dbd9d05fb64e8e34b5a/src/core/render/utils.js#L21=L40

Jacco-V avatar Jun 23 '22 09:06 Jacco-V

I've added quoted string support to the getAndRemoveConfig function and updated the tests. But I'm touching more code than I feel comfortable.

and it is kinda hard to know if things are working as expected without a working dev environment; it would be most helpful if somebody could review the changes.

Jacco-V avatar Jun 23 '22 20:06 Jacco-V

Any chance anyone of the reviewers is going to review this?

Jacco-V avatar Jul 03 '22 12:07 Jacco-V

Code has been tested and is working as intended, with the sole caveat that:

  • we cannot support escaped characters in quoted-string config-values, because by the time the str is passed to the getAndRemoveConfig() method, it has been html-escaped, possibly smartypantsified, and backslashes have been stripped. However, I expect his will not be an issue for the large majority of use cases.

Changes can be seen in action in Docsify > Guide > Helpers documentation.

Jacco-V avatar Jul 04 '22 11:07 Jacco-V

bump

Jacco-V avatar Aug 09 '22 11:08 Jacco-V

Hi, Is anyone going to review this PR? or is this functionality somehow not good enough?

Cheers,

Jacco-V avatar Aug 28 '22 11:08 Jacco-V

@Koooooo-7 Yes, I can see what you mean.

Architecturally speaking, it would be best if the str argument would be passed to the getAndRemoveConfig() function before the markdown has been transformed into html.

If this was the case, we could fully support quoted strings, including escape characters. This would mean you get clean code and the benefit of a using a syntax everybody knows.

:sidebar="In quoted strings we can normally use escaped \" characters"

However, I'm not sure it is possible to do this, without significant changes to the code base. Having said that, from an architectural point of view, it would still be the best option.

Another option would be to replace the regular-expression based 'parsing' with an actual parser, which would be able to handle escape charters even in the html formatted str argument. This option would get the quoted strings working, without addressing the architectural issue, but the end-user won't see the difference.

The other option could be as you propose: go for a different set of start of string and end of string markers. This would sidestep the problem. The code-changes would be more simple, at the cost introducing a custom format instead of using quoted strings and not addressing the architectural issue.

In the end, it is not up to me to choose which option is best suited for this project: I'm not one of the project members; I'm just a back-end guy who contributed a pull-request.

Jacco-V avatar Aug 30 '22 10:08 Jacco-V

@Jacco-V Thx for your input ! About change the regex to parser for configs is a good idea but we have not moved forward to it yet. And exposing the getAndRemoveConfig as a hook is on the road. I suppose this hook could let user to custom config tpl more flexible, we could only provide a happy path one by default.

Koooooo-7 avatar Aug 30 '22 11:08 Koooooo-7

@Jacco-V @Koooooo-7 --

I haven't tested this PR, but perhaps we can work with the escaped character limitations using one of the following approaches:

  1. Allow titles to be wrapped in single or double quotes. This allows for either quote type to be used within the string without needing to be escaped:
    :sidebar="Single 'quotes' here"
    
    :sidebar='Double "quotes" here'
    
  2. Use HTML entities
    :sidebar='Double "quotes" here'
    

FWIW, I would also change the "Customize..." sidebar labels to the following:

  • Helpers > Heading IDs
  • Helpers > Sidebar Titles

These labels align with the other sidebar labels under the "Helpers". As an added bonus, it removes the "customise vs customize" localization issue. I know that's not the goal of this PR, but hey, while we're here... :)

jhildenbiddle avatar Sep 09 '22 16:09 jhildenbiddle

@jhildenbiddle How you doing ~

About the Customize changes, I think it is a nice change to make the label more plain and understandable.

The "/' approach it sounds both fine. The more importance thing to me is the now and further enrichment configs, I think we should refactor getAndRemoveConfig to be more flexible parser(s) for configs, otherwise , the regex would be more and more complex and messy.

Koooooo-7 avatar Sep 10 '22 16:09 Koooooo-7

@jhildenbiddle This is a possibility, and it could be an intermediate solution, but it would also be another incarnation of 'sidestepping the problem', although with a better solution than introducing custom start of string and end of string markers (in my opinion).

However, by sidestepping the problem, you continue building on non-robust legacy choice and will need to add increasingly complex regular expression(s) to hammer the non-ideal setup into a working solution. So, I agree with @Koooooo-7: Going the getAndRemoveConfig hook, with a parser way, you get more flexible, more robust, and easier to maintain code.

Jacco-V avatar Sep 12 '22 09:09 Jacco-V

@Jacco-V, @Koooooo-7 --

No arguments from me about the issues with building upon problematic legacy code. My comment was intended only to propose a way to make the stated drawback of this PR -- the inability to support escaped characters in quoted-string config-values -- less of an issue so that we can deliver a solution sooner. If someone wants to take a crack at doing things the "better" way, I'm all for it.

jhildenbiddle avatar Sep 12 '22 14:09 jhildenbiddle