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

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

Open
cornfeedhobo opened this issue Jul 12, 2020 · 7 comments
Open

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

cornfeedhobo opened this issue Jul 12, 2020 · 7 comments

Comments

@cornfeedhobo
Copy link
Member

cornfeedhobo commented Jul 12, 2020

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

  • It would be nice to make these consistent
  • base seems like it should only own things used by other components, right?
  • general should be non-specific, all around general stuff, right?

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:

  • munging ls (without checking or negating the existing alias)
  • adding the irc alias
  • adding language aliases (shouldn't that be in the relevant language alias file?)

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

@cornfeedhobo cornfeedhobo changed the title Cleanup and define "general" Cleanup and define "base" and "general" Jul 12, 2020
@nwinkler
Copy link
Member

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
Copy link
Member Author

@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
Copy link
Member

nwinkler commented Sep 9, 2020

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
Copy link
Member Author

cornfeedhobo commented Sep 10, 2020

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
Copy link
Member

@cornfeedhobo wanna follow up on that?

@cornfeedhobo
Copy link
Member Author

@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
Copy link
Contributor

I'd like to suggest:

  • base == move it in to $BASH_IT/lib under helpers or utilities or something;
  • general == misc very-broad-utility simple shortcuts;
  • system == load the platform-provided thing (e.g., Bash Completion).

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants