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

aliases/general & plugin/extract #2095

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

gaelicWizard
Copy link
Contributor

@gaelicWizard gaelicWizard commented Feb 18, 2022

Description

  • use -A instead of -a for ls,
  • quote variable expansions,
  • don’t assign default expansions,
  • don’t alias piano without pianobar,
  • enable alias/bash-it in profile/default,
  • split out aliases/directory,
  • split out aliases/editor,
  • improve load warning in alias/vim,
  • move xt to plugin/extract and format the file...

Motivation and Context

Just trying to cut down on the miscellaneous stuff in aliases/general...

How Has This Been Tested?

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.

@gaelicWizard gaelicWizard changed the title aliases/general: cleanup aliases/general & plugin/extract Feb 18, 2022
@gaelicWizard gaelicWizard force-pushed the alias/general branch 6 times, most recently from f22b70d to ce281eb Compare February 18, 2022 06:20
@gaelicWizard gaelicWizard marked this pull request as ready for review February 18, 2022 06:23
source "$BASH_IT/aliases/available/bash-it.aliases.bash"
source "$BASH_IT/aliases/available/directory.aliases.bash"
Copy link
Member

Choose a reason for hiding this comment

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

loading it here wont make it load it twice in case its enabled?
we can check if the alias is already enabled, and source only if its not

Copy link
Contributor Author

@gaelicWizard gaelicWizard Feb 19, 2022

Choose a reason for hiding this comment

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

This one actually confused the heck out of me. If the user disabled it, we should not load it. If the user enabled it, we don't need to load it. How can we tell? No idea.

Copy link
Member

Choose a reason for hiding this comment

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

hmmm, I think that we should always check here if enabled, enable and source if not. If users dont want a certain alias, they should disable the general file. We can add a log here about it. I do not want to break backwards compat with this change

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds right to me too.

- use `-A` instead of `-a` for `ls`,
- quote variable expansions,
- don’t *assign* default expansions,
- don’t alias `piano` without `pianobar`,
- enable `bash-it` aliases in the default profile…
@seefood
Copy link
Contributor

seefood commented Nov 7, 2024

I say we fix the autoloading issue, merge from master and try again.

@seefood seefood added seems abandoned rattle the cage, and close if nobody wants to keep it going waiting-for-response labels Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
seems abandoned rattle the cage, and close if nobody wants to keep it going waiting-for-response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants