Bash-it / bash-it

A community Bash framework.
MIT License
14.27k stars 2.29k forks source link

Cleanup and define "base" and "general" #1640

Open cornfeedhobo opened 4 years ago

cornfeedhobo commented 4 years ago

We have base.plugin.bash and general.aliases.bash

Some of the stuff included can be pretty random. e.g. the alias includes support for pianobar (without checking for it's existence). Things like pianobar are easy to identify and break out into dedicated plugins, but more contentious to tackle are things like where to place:

So I'm going to start some PRs, but I'd also like to start a discussion and collect opinions.

nwinkler commented 4 years ago

Yes, good stuff! A lot of this is there for historic reasons and needs to be cleaned up, moved, or refactored. The pianobar and irc stuff is from way back, before I joined this project. I've left it in there since it did not really hurt, and I did not want to break people's existing installations...

Cleaning it up would be good, but would require some documentation or notification. Release notes would be nice for these kind of changes.

cornfeedhobo commented 4 years ago

@nwinkler Okay, starting with base, usage is an ambiguous name and I nominate it first for the chopping block. I've searched the codebase and don't see it used anywhere. Can I remove or rename it?

nwinkler commented 4 years ago

Good question (and sorry for the delay in getting back to you!)

I think we shouldn't remove the function, since it looks useful and we don't know how many people are using it...

Having said that, the implementation is more complex than needed, the same thing could have been achieved with an alias. The optional parameter handling is not really needed, and switching between Linux/macOS could have been done when defining the alias. This might be a candidate for moving it to an alias.

Renaming is probably a better choice, although if people are using the function, they might be confused if it's suddenly gone. Not sure what to do in this case.

cornfeedhobo commented 4 years ago

Yeah, I thought about this for a while.

Overarching $0.02; when I tell devs that I have no influence over to use bash-it, I get an earful about performance. Especially with MacOS pushing zsh, we will need to do a look at performance and that, IMHO, will necessitate organization so useless functions are not loaded. I've even been reading about dynamic built-ins and what that could look like for the backbone of bash-it.

I'd say let's move these functions that don't have clear ownership or hierarchy (base hints that other plugins might build off it, or themes might test for loaded functions). If someone comes complaining, we point them at the archived plugin (probably a bad name, but you get the idea).

Edit: yeah I also like the idea of renaming, because that would free the namespace for maintained functions. although, in this case, usage is way too general for me to sign off on a name like that these days.

NoahGorny commented 3 years ago

@cornfeedhobo wanna follow up on that?

cornfeedhobo commented 3 years ago

@NoahGorny I was waiting for more opinions. I'd like to rename or remove some of these functions, and we haven't really discussed how we want to handle that.

gaelicWizard commented 3 years ago

I'd like to suggest:

plugins/system.plugin.bash might load /etc/bashrc_${TERM_PROGRAM} or similar.

themes/base.theme.bash is super weird location. It's not a theme at all; it's just utility functions for use by themes and it's hard-coded in to bash_it.sh. It should be lib/theme.bash maybe?

plugins/base.plugin.bash should definitely be plugins/general.plugin.bash, but maybe it should be broken up in to pieces too.

In any case, I definitely vote for more smaller files so that users can load things that work for them, and not "general" with some random stuff that may or may not be useful for them. No idea what to do about backwards compatibility; might be more complexity than it's worth.