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.

Sign up for GitHub

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

Jump to bottom

lib/helpers: fixes, improvements, consolations, constellations, and a partridge in a pear tree #2061

Merged
merged 18 commits into from
Mar 1, 2022
Merged

lib/helpers: fixes, improvements, consolations, constellations, and a partridge in a pear tree #2061

merged 18 commits into from
Mar 1, 2022

Conversation

gaelicWizard
Copy link
Contributor

@gaelicWizard gaelicWizard commented Jan 25, 2022 •
edited
Loading

Description

  • @souhaiebtar's fix wrong function name call in helpers.bash #2058,
  • lib/helpers: handle some unbound positional parameters in some functions,
  • lib/helpers: handle undeclared variables in the new profile load/save functions,
  • lib/helpers: don't change directory when reloading/restarting the shell,
  • lib/search: declare variables at the top so they're never undefined.
  • lib/theme: handle unbound $remote_info.
  • lib/helper: show change log only once in _bash-it-update().
  • lib/utilities: _bash-it-component-cache*() improvements.

Motivation and Context

While working on my BATS branch, I found several codes needing fixen so here they are 😃

How Has This Been Tested?

Working on it...

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • If my change requires a change to the documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
  • I have added tests to cover my changes, and all the new and existing tests pass.

@souhaiebtar
fix wrong function name in helpers.bash
a312e5a 
when i tried to install, i got a message `_bash-it-pluralize-component` command not found; after checking `utilities.bash` the correct function name was `_bash-it-component-pluralize`
@gaelicWizard gaelicWizard force-pushed the lib/helpers branch 2 times, most recently from a312e5a to 58a9dc9 Compare January 25, 2022 21:22
@gaelicWizard gaelicWizard changed the title tests: fix wrong function name in helpers.bash DRAFT: tests: fix wrong function name in helpers.bash Jan 25, 2022
@gaelicWizard gaelicWizard force-pushed the lib/helpers branch 3 times, most recently from 9d7c998 to 2b1f121 Compare January 31, 2022 00:20
@gaelicWizard gaelicWizard changed the title DRAFT: tests: fix wrong function name in helpers.bash DRAFT: lib/helpers: minor fixes Feb 2, 2022
@gaelicWizard gaelicWizard changed the title DRAFT: lib/helpers: minor fixes lib/helpers: minor fixes Feb 2, 2022
@gaelicWizard gaelicWizard marked this pull request as ready for review February 2, 2022 20:16
@gaelicWizard gaelicWizard changed the title lib/helpers: minor fixes lib: minor fixes Feb 2, 2022
@gaelicWizard gaelicWizard changed the title lib: minor fixes lib: minor parameter fixes Feb 2, 2022
gaelicWizard added a commit to gaelicWizard/bash-it that referenced this pull request Feb 9, 2022
@gaelicWizard
install: TLC and enhancements
4ba6185 
- Allow direct installation by running `bash -c "$(curl -s https://raw.github.com/bash-it/bash-it/master/install.sh)"`
- This depends on Bash-it#2061 due to unbound variables and a function name typo in `lib/helpers`
@gaelicWizard gaelicWizard force-pushed the lib/helpers branch 4 times, most recently from 3483027 to beea708 Compare February 9, 2022 05:44
@gaelicWizard gaelicWizard changed the title lib: minor parameter fixes lib/helpers: minor parameter fixes Feb 9, 2022
@gaelicWizard gaelicWizard changed the title lib/helpers: minor parameter fixes lib/helpers: parameter fixes Feb 9, 2022
@gaelicWizard gaelicWizard force-pushed the lib/helpers branch 2 times, most recently from 24881eb to 377f92b Compare February 12, 2022 07:00
NoahGorny
NoahGorny requested changes Feb 13, 2022
View reviewed changes
Copy link
Member

@NoahGorny NoahGorny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty well made, I had a few comments

lib/helpers.bash Show resolved Hide resolved
lib/helpers.bash Outdated Show resolved Hide resolved
@gaelicWizard gaelicWizard force-pushed the lib/helpers branch 8 times, most recently from c505603 to f2bad2a Compare February 18, 2022 10:46
tsiflimagas
tsiflimagas reviewed Feb 18, 2022
View reviewed changes
lib/utilities.bash Outdated
Comment on lines 163 to 164
enabled=($(compgen -G "$BASH_IT/$component_dir/enabled/*.$component.bash")) ||
enabled=($(compgen -G "$BASH_IT/enabled/*.$component.bash"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, haha, you picked that? 😅 I had made some very sketchy changes, which were half broken, but then I got in lack of time, so I pushed them to continue with those later. I see you reworked them to perfection though, so it's great! Also, unsetting array subscripts, damn, I never thought of that. It's perfect. I always did arr="${arr//i}", which would leave an empty member.
About compgen, thanks for indicating that, it's important to me when I learn something from someone who's better at it. Usually, when I'm sure that the files I'm working with won't have something weird in their names, I let myself be more incautious. But yeah, it's better to always avoid bad practices (especially in bash it seems to be too easy to slip).

NoahGorny
NoahGorny approved these changes Feb 19, 2022
View reviewed changes
Copy link
Member

@NoahGorny NoahGorny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ready, there are some changes that are just not related, but they are simple enough im inclined to include them with this PR

@NoahGorny NoahGorny merged commit 34bc37c into Bash-it:master Mar 1, 2022
@gaelicWizard gaelicWizard deleted the lib/helpers branch March 2, 2022 02:35
This was referenced Mar 3, 2022
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsiflimagas tsiflimagas tsiflimagas left review comments

@NoahGorny NoahGorny NoahGorny approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gaelicWizard @NoahGorny @tsiflimagas @souhaiebtar