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

Tidy up settings script #2593

Closed
wants to merge 11 commits into from
Closed

Tidy up settings script #2593

wants to merge 11 commits into from

Conversation

trn1ty
Copy link

@trn1ty trn1ty commented May 20, 2024

I noticed there was some tricky IFS usage to separate file names, but shell can do that in a for loop anyway, so I went ahead and cleaned all that up as well as making some other small changes for readability. Functionality should be equivalent except that settings help shows a help screen now.

@Botspot
Copy link
Owner

Botspot commented May 20, 2024

echo and printf are both bash builtins, so I am not aware of any meaningful performance difference between the two.
So when you use printf just like echo, is there a reason for that?

Why exchange this:
echo "$output"
for this?
printf '%s\n' "$output"
Do you find that to be more readable? Or is it somehow better practice?

We like code readability, and some of your changes here seem to make the code harder to read.

@theofficialgman theofficialgman marked this pull request as draft May 30, 2024 05:26
@theofficialgman theofficialgman added the waiting on response Further information is requested from the issue owner by the pi-apps devs label Jul 26, 2024
@theofficialgman
Copy link
Collaborator

Closing as stale as the author has not replied to comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting on response Further information is requested from the issue owner by the pi-apps devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants