search

goolord / alpha-nvim

a lua powered greeter like vim-startify / dashboard-nvim
MIT License
1.84k stars 109 forks source link

refactor: cleanup opts structure #80

Closed kylo252 closed 2 years ago

kylo252 commented 2 years ago

Breaking: opts.opts.FOO is now accessed directly with opts.FOO instead

This was mainly used with some options, such as noautocmd and margin

-- old
dashboard.opts.opts.margin = 5

-- new
dashboard.opts.margin = 5

-- old
startify.opts.opts.redraw_on_resize = false

-- new
startify.opts.redraw_on_resize = false

The main benefits of this change is clarity and shorter/simpler parsing.

goolord commented 2 years ago

i'm a bit conflicted, it's a silly name (i am hilariously bad at naming things lol) but the opts.opts table is meant to indicate that those options are opt ional :)

imo that makes it easier to copy+paste code around without reading docs

perhaps it would be more apt to rename the parent opts table config or something?

kylo252 commented 2 years ago

but the opts.opts table is meant to indicate that those options are opt ional :)

I get what you mean but they're already part of opts

imo that makes it easier to copy+paste code around without reading docs

It still is in this case, I don't see how that is changing that part.

perhaps it would be more apt to rename the parent opts table config or something?

The better approach from a refactoring pov, would be to rename the entire opts table, which is an unfortunate name since it's not really optional. but now we'd be really introducing a breaking change for everyone, so I'm thinking you mean dashboard.opts.config instead of dashboard.opts.opts. In that case, I thought about calling it global_opts instead.

goolord commented 2 years ago

i was thinking dashboard.config.opts

kylo252 commented 2 years ago

i was thinking dashboard.config.opts

Yeah, but that's the bigger breaking change that I was talking about, it would require a deprecation notice and I'm not exactly sure how to handle it on the fly instead of just bailing out and telling the user to use the new syntax.

To be clear, I'm in favor of your suggestion.

goolord commented 2 years ago

well, we could export opts = config, which would avoid any damage to the end user i think. then a migration plan would look like

  1. replace mentions of opts with config in the docs
  2. emit a deprecation warning when opts is used
  3. after X time has passed, remove opts with a breaking! commit so packer users are notified