Skip to content

Conversation

cartercanedy
Copy link
Contributor

@cartercanedy cartercanedy commented Sep 10, 2024

Use dash instead of bash for inlined application-setup commands

Type of Change

  • New feature
  • Bug fix
  • Documentation Update
  • Refactoring
  • Hotfix
  • Security patch
  • UI/UX improvement

Description

The setup scripts for both mybash & Chris's custom neovim setup use ANSI escapes to color text, but bash isn't posix compliant in the way that the echo command handles escapes. This change allows for proper presentation of the commands as they emit notifications without distracting escape sequences being emitted without being interpreted.

Testing

Very simple change, just verified that the commands run with the escape sequences being interpreted properly

Impact

Removes a lot of superfluous escape sequences and makes the command output more readable.

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no errors/warnings/merge conflicts.

@cartercanedy cartercanedy force-pushed the use-dash-instead-of-bash branch from 3a46337 to 5b5743a Compare September 10, 2024 18:11
@cartercanedy cartercanedy force-pushed the use-dash-instead-of-bash branch from ff854e0 to 52b5648 Compare September 10, 2024 18:18
@ghost

This comment was marked as off-topic.

@cartercanedy
Copy link
Contributor Author

It's probably better that they stay where they are since they're definitely going to be changing. They're posix enough that they're designed to be run with dash anyway, hence the echo issues

@ghost

This comment was marked as off-topic.

@cartercanedy
Copy link
Contributor Author

Considering that one of them is a bash config, it should be fine...

@ghost

This comment was marked as off-topic.

@cartercanedy
Copy link
Contributor Author

cartercanedy commented Sep 11, 2024

The setup script for mybash* has no bashisms

@ghost

This comment was marked as off-topic.

@ghost

This comment was marked as off-topic.

@cartercanedy
Copy link
Contributor Author

cartercanedy commented Sep 11, 2024

The setup script for mybash* has no bashisms

The original script in CTT's repo does have bashisms

image

This warning is part of* the reason that I made the switch. Bash doesn't render escapes by default, which isn't posix compliant, but this is printed normally using dash or any other shell

@ghost
Copy link

ghost commented Sep 11, 2024

The setup script for mybash* has no bashisms

The original script in CTT's repo does have bashisms
image

This warning is pay off the reason that I made the switch. Bash doesn't render escapes by default, which isn't posix compliant, but this is printed normally using dash or any other shell

I see. Could you give me opinions on #335 ?

@ChrisTitusTech
Copy link
Owner

Sorry for the inconvenience. We had a massive restructure of the codebase to improve future development. Because of this can you update your PR to the new structure. Thank you for your assistance and contribution.

@cartercanedy cartercanedy deleted the use-dash-instead-of-bash branch September 24, 2024 16:13
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.

2 participants