Skip to content

Conversation

@gaelicWizard
Copy link
Contributor

@gaelicWizard gaelicWizard commented Sep 20, 2021

Description

Simplifies the flow of aliases/vim by short circuiting instead of doing a whole if block and reducing weirds.

Motivation and Context

These were part of my _command_exists branch, but was out of scope.

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 marked this pull request as ready for review September 20, 2021 19:59
@cornfeedhobo
Copy link
Member

cornfeedhobo commented Sep 21, 2021

do we want to account for the condition where the user doesn't have vim installed but does have nvim and has aliased that to vim?

@davidpfarrell
Copy link
Contributor

do we want to account for the condition where the user doesn't have vim installed but does have nvim and has aliased that to vim?

I think _command_exists vim will pickup the alias (just confirmed in my local), so I think existing vim logic generally works in this case.

Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

I don't know if the OS switch was necessarily a bad thing, but I think the _command_exists check will short-circuit out easily enough when not present.

Approved !

@NoahGorny NoahGorny merged commit d6c4c0c into Bash-it:master Sep 23, 2021
@gaelicWizard gaelicWizard deleted the alias-vim branch September 23, 2021 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants