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

DietPi-Banner | Adds large style banner #5113

Merged
merged 10 commits into from
Jan 3, 2022
Merged

DietPi-Banner | Adds large style banner #5113

merged 10 commits into from
Jan 3, 2022

Conversation

matellis
Copy link
Contributor

Fix for feature request #5110

Status: Work in Progress

  • Adds a large style hostname header in the banner using figlet, simple change
  • I don't know how to ensure figlet is installed or to install it when required, or even if figlet is allowed on a light install like DietPi

Commit List

  • dietpi/func/dietpi_banner | Adds large style banner to the top, configurable via the menu (as option 14)

End Result

Screencap

@Joulinar Joulinar changed the title Dev DietPi-Banner | Adds large style banner Dec 26, 2021
@Joulinar Joulinar requested a review from MichaIng December 26, 2021 23:28
@MichaIng MichaIng linked an issue Dec 27, 2021 that may be closed by this pull request
@MichaIng
Copy link
Owner

Many thanks for your suggestion. Looks good, though I suggest to prompt an info and install figlet automatically when this option is selected. Currently it would silently do nothing until figlet is manually installed.

Also, are there alternatives to figlet? I know toilet, which has some more features, including colours and unicode, though nothing we'd require here. I personally would prefer a native shell script so that no external binary needs to be called, also reading the hostname directly from /etc/hostname instead.

@matellis
Copy link
Contributor Author

  1. Good idea on using /etc/hostname directly, I've made that change.

  2. As for replacing figlet with a native shell script, I found a project called banner by @rern that does the job, but it's 700+ lines of bash. I'm sure we could thin it down a little but it's still longer than the entire dietpi-banner script itself. If you have any ideas here which could help, or you don't mind tripling the length of the existing script then we could use that.

  3. I had a look at toilet which, at first glance, appears newer and more richly featured than figlet. This also produces a bigger footprint and a wider range of things which need to be configured or installed, e.g. fonts. In terms of features It feels like banner -> figlet -> toilet is the order from fewest to most. If the goal is to reduce footprint, then toilet feels like 'too much' and we should stay with figlet.

  4. The last option would be to use the 170-lines of C source for the original pre-figlet app, originally called newban, strip it down and use that. However, after a reasonable search I wasn't able to track that down, it was originally released in 1991 and may not be usable.

If the decision is made to do 2, 3 or 4, can you point me at an example of "prompting an info" to install a package?

@MichaIng
Copy link
Owner

In theory we could take the characters allowed in hostnames from the font matrix from here or similar open source scripts and put them into an own script to source or to call (so that it returns the output). The logic to concatenate the input character slices line by line is not too complicated. It has the benefit that we can tune each character as we like, change the style etc. I'd then simply go mono colour and in case allow to change it for the whole text block.

Agreed that toilet brings much more dependencies and overhead which we do not need, to in case we stay with an external tool, figlet seems preferable.

I need to apply some fixes to existing features but will then do a quick port of that shell script and compare performance to figlet. I guess best would be achieved indeed when we'd put the character arrays into an own script, together with a function to translate the input string, sourcing that (instead of executing) from dietpi-banner if the feature is enabled. This way the script does not significantly has its size increased for users which do not use it.

@MichaIng MichaIng added this to the v8.0 milestone Jan 2, 2022
@MichaIng
Copy link
Owner

MichaIng commented Jan 2, 2022

Okay, I created a shell script now, much like yours, but with minimal logic for what we need + lower case characters: https://github.com/MichaIng/hacks/blob/main/dietpi-print_large

Great idea to generate the character arrays with a script from figlet as well. I missed at first that you created an own fork of the original script with such a dedicated translator script. I adopted the idea as well, which can be used with other figlet-like tools or new characters when needed: https://github.com/MichaIng/hacks/blob/main/dietpi-figlet_to_shell

Took a while until I got it performing significantly better than figlet itself. Since looping through <<< "$input" forcefully implies a trailing new line, read by read, I first used < <(echo -n "$input"), but that step somehow enormously increases execution time 😕:

2022-01-02 20:29:31 root@VM-Bullseye:/tmp# time for i in {1..100}; do while read -r line; do :; done <<< ''; done

real    0m0.007s
user    0m0.000s
sys     0m0.007s
2022-01-02 20:30:17 root@VM-Bullseye:/tmp# time for i in {1..100}; do while read -r line; do :; done < <(echo -n ''); done

real    0m0.865s
user    0m0.155s
sys     0m0.757s

Little confusing since all is still pure internal bash syntax/commands. However, [[ $line ]] || continue works well as alternative to skip the trailing new line without any noticeable execution time added.

The script can be executed or sourced (all variables are declared as local ones, only the wrapper function remains declared in parent shell), in both cases printing the first input argument in large fonts and an error message instead if any invalid character is contained. So we can add it as dedicated script to DietPi, probably finding a further use our end or users may detect and find it useful. And for best performance it can then be sourced from dietpi-banner:

. /boot/dietpi/func/dietpi-print_large "$(</etc/hostname)"

Shall we implement it like that?

Btw, the (small) hostname -f print (for consistency we may switch to $(</etc/hostname)) as well) probably can be omitted automatically when large hostname is enabled?

@matellis
Copy link
Contributor Author

matellis commented Jan 2, 2022

This is great, yes to hostname being ommitted the second time.

Would you like me to adapt the PR to use your new script? I think that would answer all feedback then, right?

@MichaIng
Copy link
Owner

MichaIng commented Jan 2, 2022

Would you like me to adapt the PR to use your new script?

Yes please do so when you find time. With the maintainer edit tick set, I can edit scripts changed in this PR but not add new ones to your branch 😉.

I think that would answer all feedback then, right?

Yes, as it's several times faster than figlet, adds less data/dependencies, and we get gain the flexibility to tune character styles when we want, this is preferable I'd say 🙂.

@matellis
Copy link
Contributor Author

matellis commented Jan 2, 2022

Your script has been added, displacing figlet. Works well.

Then I went onto making things more consistent and reconsidered some of our thoughts around hostname. The other two options aren't the same thing: the header uses short -s whereas the other two options use full -f or NIS n domain names. I didn't want to remove either, as folks relying on these would be forced to change.

Looking at this, I changed the code in this PR to use hostname -s to be consistent with other uses of hostname, and also to preserve whatever special logic any future distro's might put into hostname and/or /etc/hostname. Happy to revisit this if you feel it's worth it.

At this point, thanks to your prompting, this feels like a tight & tidy patch. The only thing to consider now is: do we make it the default going forwards?

@MichaIng
Copy link
Owner

MichaIng commented Jan 2, 2022

I added a change so that the script is now sourced instead of executed, to skip the additional bash process spawn. Also /etc/hostname is now read for both small and large hostname print. The NIS domain name is a different case of course.

Works great. I'm wondering whether the extra new line is required. Though when using p or q or similar lower case characters which use the last output line, it is indeed a little too close to the other outputs below 🤔.

EDIT: And now the small hostname is skipped when the large one is printed already.

and fix accidentally swapped NIS/YP domainname with hostname
@matellis
Copy link
Contributor Author

matellis commented Jan 3, 2022

Feels like a matter of personal choice and easily changed one way or the other. I added the extra line to make it symmetric top to bottom, but I have no special feelings either way.

FWIW, in my personal setup I have fortune at the bottom too... Here's a screen cap:

@MichaIng
Copy link
Owner

MichaIng commented Jan 3, 2022

Lovely 🙂.

You are right, with the extra newline it looks good in all cases, skipping that with some lower case characters it doesn't look nice. When using this option, obviously terminal height isn't a problem.

One more think we may want to implement, is a better full font line break when terminal console is too small. This would basically mean to split the 6 output lines at console with, and if any line is too long, add array indices 7-11 with the overflow, or empty string. But usually hostnames are not that long, so let's skip this for now and add it in a future release if either users report related ugly line breaks or we are in mood to add the additional logic.

@MichaIng
Copy link
Owner

MichaIng commented Jan 3, 2022

Merging this now, we can tune at any time, e.g. as of feedback during beta phase which starts soon.

@MichaIng MichaIng merged commit 94d02cb into MichaIng:dev Jan 3, 2022
MichaIng added a commit that referenced this pull request Jan 3, 2022
- CHANGELOG | DietPi-Print_large: This new script has been added which can be executed or sourced from /boot/dietpi/func/dietpi-print_large to print the string passed via first argument in large figlet style fonts. It currently only supports the characters a-z, A-Z, 0-9, dot and dash, i.e. those commonly allowed in hostnames.
- CHANGELOG | DietPi-Banner: Added an option to print the system's hostname in large figlet style fonts, right below the banner header. In case it is enabled as well, the regular/small hostname line will then be skipped. Many thanks to @matellis for implementing this feature: #5113
@MichaIng
Copy link
Owner

MichaIng commented Jan 3, 2022

Changelog: e82e6ef

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DietPi-Banner | Large Format Hostname
3 participants