nvim-telescope / telescope-smart-history.nvim

A history implementation that memorizes prompt input for a specific context
MIT License
141 stars 5 forks source link

move config options from default to extensions #5

Open JoseConseco opened 2 years ago

JoseConseco commented 2 years ago

most tele extensions put their config in extensions table: image I wonder maybe tele-shart history should do this too? Its then easier to see that this config is from extension.

Conni2461 commented 2 years ago

But this configuration is part of telescope.actions.history and will be loaded there. All this extension does is provide a class that overrides the base history class. Similar to how telescope.actions.history.get_simple_history does (which is the history model that is builtin to telescope).

The base class loads the configuration, there is no configuration loading here.

We could do an additional config section thought with the path and limit options but in the end i would just pass them to defaults.history.path and defaults.history.limit. And this solution would just generate code duplication and is just unnecessary.

Please read :help telescope.defaults.history and :help telescope.actions.history (which contains the documentation for the base class)

JoseConseco commented 2 years ago

Ok, thanks for info. I know that it would duplicate the history settings, but IMO from user point of view - it is easier to manage extensions configs, if they are all placed in one table (extensions sub table). Imagine having dozen of extensions, and some of them would override defaults settings - then without comments it could be hard to tell what option is responsible for which plugin. If you removed extension X, then imagine having to go through your defaults - and having too correct them. With extensions.smart_history = {} - you could just remove this one part (and defaults.history would take over) . Let me know if you consider this, if not then I can close the issue.