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

Allow overriding landscape theme sidebar config #4723

Closed
wants to merge 1 commit into from

Conversation

FossPrime
Copy link

@FossPrime FossPrime commented Jun 26, 2021

Landscape provides a fully loaded config, there is no clean way to get a minimal config without hard forking the theme.

This allows a cleaner way over overriding theme configs.

#3890
#4154
hexojs/hexo-theme-landscape#102

In most inheritance models, class methods are fully superseded by the extending class, pyYaml supports this model.

Alternatively we could use a database migration model, where the config has a theme version and schema version and merges are done based on that.

This PR just works with the present situation.

How to test

git clone -b BRANCH https://github.com/USER/hexo.git
cd hexo
npm install
npm test

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.
  • was written on a phone.

Landscape provides a fully loaded config, there is no clean way to get a minimal config without **hard** forking the theme.

This allows a cleaner way over overriding theme configs.

In most inheritance models, class methods are fully superseded by the extending class, pyYaml supports this model.

Alternatively we could use a database migration model, where the config has a theme version and schema version and merges are done based on that.

This PR just works with the present situation.
@FossPrime
Copy link
Author

It's clear this isn't enough. What's needed is a major update full of hooks and a two tier theme yml. One one internal and override-discouraged, one external and fully override able. As well as hooks for late-cascade scss styles. Same for a JavaScript module hook or 10.

Forking is always worse than override-able thing we can provide.

@FossPrime FossPrime closed this Jun 27, 2021
@stevenjoezhang
Copy link
Member

We need to update the default _config.yml of landscape and comment out some configuration options @hexojs/core

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.

2 participants