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

Replace remaining echo -e statement #403

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

nnyyxxxx
Copy link
Contributor

@nnyyxxxx nnyyxxxx commented Sep 16, 2024

Type of Change

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • My changes generate no errors/warnings/merge conflicts.

@nnyyxxxx nnyyxxxx changed the title Fix last bashism (missed it) 😂 Fix last bashism (missed it) 🤣 Sep 16, 2024
@guruswarupa
Copy link
Contributor

haha happens a lot of times :)

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Sep 16, 2024

haha happens a lot of times :)

This one was really obscure, didn't think to look in any of the start scripts; my mistake.

@jeevithakannan2
Copy link
Contributor

Change common script line 37

echo -e "${RED}Failed to install AUR helper.${RC}"

@nnyyxxxx
Copy link
Contributor Author

Change common script line 37

echo -e "${RED}Failed to install AUR helper.${RC}"

No need, #382 does this already.

@jeevithakannan2
Copy link
Contributor

read -s password

Will not work in wifi-control.sh line 139

@jeevithakannan2
Copy link
Contributor

use

stty -echo
read password
stty echo

@nnyyxxxx
Copy link
Contributor Author

@jeevithakannan2 If you want to implement that then make a PR and gloss over the util scripts.

@jeevithakannan2
Copy link
Contributor

@jeevithakannan2 Should be fine.

read -s is not posix compliant will not work in debian 11 and 12

@jeevithakannan2
Copy link
Contributor

@jeevithakannan2 If you want to implement that then make a PR and gloss over the util scripts.

If you're creating a PR then make sure it does everything what you have described !!

@nnyyxxxx
Copy link
Contributor Author

@jeevithakannan2 If you want to implement that then make a PR and gloss over the util scripts.

If you're creating a PR then make sure it does everything what you have described !!

A different PR would have to be made separate from this for every fixed script. It is 2 AM where I am at and cannot fix that issue ATM.

@nnyyxxxx nnyyxxxx changed the title Fix last bashism (missed it) 🤣 Fix semi-last bashism (missed it) 🤣 Sep 16, 2024
@jeevithakannan2
Copy link
Contributor

@jeevithakannan2 If you want to implement that then make a PR and gloss over the util scripts.

If you're creating a PR then make sure it does everything what you have described !!

A different PR would have to be made separate from this for every fixed script. It is 2 AM where I am at and cannot fix that issue ATM.

Then you should be putting this PR as draft and finish it later rather than debating with people that are doing their job

@nnyyxxxx
Copy link
Contributor Author

@jeevithakannan2 If you want to implement that then make a PR and gloss over the util scripts.

If you're creating a PR then make sure it does everything what you have described !!

A different PR would have to be made separate from this for every fixed script. It is 2 AM where I am at and cannot fix that issue ATM.

Then you should be putting this PR as draft and finish it later rather than debating with people that are doing their job

Not sure what you mean, I just recommended that this PR should be split up into multiple e.g. for every fix implemented, no need to be rude about it..

@nnyyxxxx
Copy link
Contributor Author

I'll do this TMR when I have time.

@nnyyxxxx nnyyxxxx marked this pull request as draft September 16, 2024 06:23
@nnyyxxxx nnyyxxxx changed the title Fix semi-last bashism (missed it) 🤣 Remove remaining read -s (oranythiing other than -r) bashisms Sep 16, 2024
@nnyyxxxx nnyyxxxx changed the title Remove remaining read -s (oranythiing other than -r) bashisms Remove remaining read -s (or anything other than -r) bashisms Sep 16, 2024
@nnyyxxxx nnyyxxxx changed the title Remove remaining read -s (or anything other than -r) bashisms Replace remaining echo -e statement Sep 17, 2024
@nnyyxxxx nnyyxxxx marked this pull request as ready for review September 17, 2024 16:02
@nnyyxxxx
Copy link
Contributor Author

I've changed my view on this; I have given the task to two fellow contributors as discussed in the discord server, for now this will just be cleaning up the last echo -e :)

@ChrisTitusTech ChrisTitusTech merged commit 19b5336 into ChrisTitusTech:main Sep 18, 2024
3 checks passed
@nnyyxxxx nnyyxxxx deleted the testing-4 branch September 18, 2024 19:12
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.

Bashism still exists in start scripts
4 participants