Skip to content

feat: support config helper multi keys if supported #2571

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

Merged
merged 5 commits into from
Jul 22, 2025

Conversation

Koooooo-7
Copy link
Member

@Koooooo-7 Koooooo-7 commented Jul 21, 2025

Summary

An enhancement for helper configs which support multi values, it will be retrieved all.

        "[alt text](http://url ':class=someCssClass :class=anotherCssClass')",

It is short term goal to find middle ground to solve the problem that allowing config a key multi times.
For single value support, it keeps the consistency of current config behaviors and we docs them as single value configs either. So there is no breaking change introduced.

A further assumption is making new docsify config system to support more flexible configuration.
Trace on #2476

Related issue, if any:

What kind of change does this PR introduce?

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed

Does this PR introduce a breaking change?

No

Yes
No

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge

Copy link

vercel bot commented Jul 21, 2025

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

Name Status Preview Comments Updated (UTC)
docsify-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2025 2:40am

@Koooooo-7 Koooooo-7 requested a review from paulhibbitts July 21, 2025 15:59
@Koooooo-7 Koooooo-7 marked this pull request as draft July 21, 2025 16:05
@Koooooo-7 Koooooo-7 marked this pull request as ready for review July 21, 2025 16:25
@Koooooo-7 Koooooo-7 requested a review from a team July 21, 2025 16:25
@paulhibbitts
Copy link
Collaborator

paulhibbitts commented Jul 21, 2025

Hi @Koooooo-7 , thanks very much for taking a go at this! I've made a little test site using the PR Preview:
https://paulhibbitts.github.io/docsify-v5-multiple-value-config-test/#/multiple-values-example

An image with two classes worked like a charm. Is this feature also expected to work for text links? I ask because with v5 there are now two classes for defining buttons, button + primary and button + secondary.

Hope I did everything with in my test page 🙂

PS - GitHub renders the page with the markup quite cleanly still: https://github.com/paulhibbitts/docsify-v5-multiple-value-config-test/blob/main/multiple-values-example.md

@Koooooo-7
Copy link
Member Author

Hi @Koooooo-7 , thanks very much for taking a go at this! I've made a little test site using the PR Preview: https://paulhibbitts.github.io/docsify-v5-multiple-value-config-test/#/multiple-values-example

An image with two classes worked like a charm. Is this feature also expected to work for text links? I ask because with v5 there are now two classes for defining buttons, button + primary and button + secondary.

Yep, I think this should work either.

[logo-link](https://docsify.js.org/_media/icon.svg ':id=someCssId :class=someCssClass :target=_self :class=anotherCssClass :disabled')

Hope I did everything with in my test page 🙂

Your working as fast as the rocket shoot ! 🚀

@paulhibbitts
Copy link
Collaborator

Thats awesome @Koooooo-7 , this initial support for multiple classes is VERY useful 🙌🏻 Thank you!

paulhibbitts
paulhibbitts previously approved these changes Jul 21, 2025
Copy link
Collaborator

@paulhibbitts paulhibbitts left a comment

Choose a reason for hiding this comment

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

This is looking really good! The PR Preview has been tested on the following page: https://paulhibbitts.github.io/docsify-v5-multiple-value-config-test/#/multiple-values-example

sy-records
sy-records previously approved these changes Jul 22, 2025
Copy link
Member

@sy-records sy-records left a comment

Choose a reason for hiding this comment

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

LGTM.

Co-authored-by: Luffy <lufei@docsifyjs.org>
@sy-records
Copy link
Member

Should we only allow class?

![logo](https://docsify.js.org/_media/icon.svg ‘:id=someCssId :id=id2’)

@sy-records sy-records changed the title update: support config helper multi keys if supported feat: support config helper multi keys if supported Jul 22, 2025
@Koooooo-7
Copy link
Member Author

Koooooo-7 commented Jul 22, 2025

Should we only allow class?

![logo](https://docsify.js.org/_media/icon.svg ‘:id=someCssId :id=id2’)

Currently, in our scope, only class are necessary to supported multi values.
Technically, it allows all config to config as multi times/values.
It depends on what the attr made for. Such as the id, there make no sense to config 2 id.
And we could/have doc all configs how to config, and which configs support multi values to users.
(We don't have guideline to say we support id like #Title :id=id1 :id=id2 in our document)

Personally, I think it is okay cuz user needs follow the rules. and if the config not work as expected, they can be aware of that either.
The config validation (config non exist key, config invalid value, config key with multi values but the key does not support ..etc) we could make as another topic, if necessary.
WDYT?

@Koooooo-7 Koooooo-7 merged commit a2f734f into develop Jul 22, 2025
8 checks passed
@Koooooo-7 Koooooo-7 deleted the config-enhance-clz branch July 22, 2025 04:24
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.

3 participants