Motd updates #8

agnostic-apollo · 2022-08-13T02:35:30Z

Fixes the word wrap issues of dynamic motd and other additions. Check commits.

The aarch64 build is attached, install with dpkg -i --force-confask --force-confnew /sdcard/Download/termux-tools_1.26.0-1_all.deb

Related pull requests termux/termux-packages#11250 and termux/termux-packages#11294

cc: @TomJo2000 @HardcodedCat

termux-tools_1.26.0-1_all.zip

TomJo2000 · 2022-08-13T09:54:36Z

Thank you for taking the time to help out with this.
I did a bit of work on this over the week on my own, but I just started a new internship and I haven't had the time to get it ready for a pull request yet.

My version is using Heredocs and the $COLUMNS environment variable to dynamically achieve wrap detection.

I'll leave comments at the appropriate places as I'm reviewing these changes.
Again, I'm sorry I haven't been able to be more involved in this.

TomJo2000

LGTM.

TomJo2000 · 2022-08-13T10:12:34Z

motds/motd.sh.in

+# Setup TERMUX_APP_PACKAGE_MANAGER
+source "@TERMUX_PREFIX@/bin/termux-setup-package-manager" || exit 1
+
+terminal_width="$(stty size | cut -d" " -f2)"


Terminal width can be determined by simply reading the value of $COLUMNS.
Unless that environment variable is unavailable for some reason in this context, in which case please inform me so I can fix this in my own version.

$COLUMS is not set if bash shell is not interactive, like when motd is executed, unless -i is passed, which creates other problems. It can still be set if checkwinsize is set, but requires starting a sub shell. stty is posix and should be fine to use.

https://stackoverflow.com/a/563592

Ah that makes sense, I did have checkwinsize set, but it's probably better to use stty in this context.

TomJo2000 · 2022-08-13T10:20:27Z

motds/motd.sh.in

+source "@TERMUX_PREFIX@/bin/termux-setup-package-manager" || exit 1
+
+terminal_width="$(stty size | cut -d" " -f2)"
+if [[ "$terminal_width" =~ ^[0-9]+$ ]] && [ "$terminal_width" -gt 60 ]; then


Instead of using a fixed range for the maximum column width of a line on the motd we can evaluate the maximal line length dynamically using something like this.

len_motd=$(echo "$motd_logo" | sed -E "s/\\\e\[[0-9;]+m//g" | wc -L) if (($COLUMNS > $len_motd)) then echo -ne "$motd_logo"

We read in the motd, strip out any escape sequences, and pass it to wc -L which returns the column count of the longest line.

With $motd_logo being a Heredoc/variable containing the part of the motd liable to being distorted by wrapping

I am aware of that, but no need to do such processing and calling additional external binaries like sed and wc. If motd is updated, the commiter can adjust size. Moreover, 60 is a good default for column width for motd anyways.

Yeah that's a reasonable concern.

TomJo2000 · 2022-08-13T10:25:52Z

motds/motd.sh.in

+terminal_width="$(stty size | cut -d" " -f2)"
+if [[ "$terminal_width" =~ ^[0-9]+$ ]] && [ "$terminal_width" -gt 60 ]; then
+
+    motd="


As mentioned above utilizing a Heredoc may be desirable in the interest of cleanliness and maintainability.

# Store $IFS for safekeeping OLD_IFS=$IFS # Logo portion of the MOTD. IFS='' read -rd '' motd_logo << LOGO_EOF \e[47m \e[0m \e[1mWelcome to Termux!\e[0m \e[47m \e[0m \e[47m\e[47m \e[0m \e[47m \e[0m \e[47m \e[0m \e[47m \e[0m \e[1mDocs:\e[0m \e[4mtermux.dev/docs\e[0m \e[47m \e[0m \e[47m \e[0m \e[47m \e[0m \e[1mGitter:\e[0m \e[4mgitter.im/termux/termux\e[0m \e[47m \e[0m \e[47m \e[0m \e[1mCommunity:\e[0m \e[4mtermux.dev/community\e[0m \e[47m \e[0m \e[47m\e[47m \e[0m \e[47m \e[0m \e[1mTermux version:\e[0m ${TERMUX_VERSION-Unknown} LOGO_EOF

Storing the initial $IFS may be necessary if a user has set a custom Input Field Separator.

No need for heredoc, since it is slower since it requires creation of temp file and also additional utils like read or cat. We are also appending motd variable, so current way is better.

Hmm didn't think of that.

TomJo2000 · 2022-08-13T10:27:10Z

motds/motd.sh.in

+ \e[47m                \e[0m  \e[1mTermux version:\e[0m ${TERMUX_VERSION-Unknown}
+"
+
+    motd_indent="                   "


Storing the indent of the motd as a separate variable is a great idea and I will be adopting it into my version as it simplifies handling the indentation significantly.

TomJo2000 · 2022-08-13T10:30:11Z

motds/motd.sh.in

+
+else
+
+    motd="


Same rationale as above regarding Heredocs for cleanliness and maintainability applies here, and to subsequent motd parts.

TomJo2000 · 2022-08-13T10:38:59Z

scripts/login.in

-	elif [ -f @TERMUX_PREFIX@/etc/motd ]; then
-		cat @TERMUX_PREFIX@/etc/motd
+	# Use user defined dynamic motd file if it exists
+	if [ -f ~/.termux/motd.sh ] && [ -f @TERMUX_PREFIX@/bin/bash ] && [ -x @TERMUX_PREFIX@/bin/bash ]; then


Suggested change

if [ -f ~/.termux/motd.sh ] && [ -f @TERMUX_PREFIX@/bin/bash ] && [ -x @TERMUX_PREFIX@/bin/bash ]; then

if [ -f ~/.termux/motd.sh ] && [ -x @TERMUX_PREFIX@/bin/bash ]; then

Testing for just executability is sufficient as the file needs to exist to be executable,
thus [ -x @TERMUX_PREFIX@/bin/bash ] should be sufficient to test for both existence and executability.

-f checks if bash is a regular file and not a directory. -x won't check that.

Oh right.
I suppose there could be a @TERMUX_PREFIX@/bin/bash/ directory.

TomJo2000 · 2022-08-13T10:40:59Z

motds/motd.sh.in

+    motd_indent=""
+fi
+
+motd+="


Concatenating the motd variable with applicable sections is a significantly more elegant approach than my current solution, I will will be adopting it.

motds/Makefile.am

agnostic-apollo · 2022-08-13T11:35:04Z

Also forgot to mention that environmental variable way for custom motd file path and disabling showing icon is not gonna work. Idiot me didn't consider that they won't be set in login script before shells are called to load rc files. Could use config or properties files, but current way seems fine.

Also need to merge that else and if for default motd file in login script.

kfatyuip

It seems that motd can only be determined when logging in

agnostic-apollo · 2022-08-13T12:53:20Z

It seems that motd can only be determined when logging in

What?

kfatyuip · 2022-08-13T12:59:46Z

It seems that motd can only be determined when logging in

What?

If the motd is printed when logging in, maybe it can not be changed by zooming : (

agnostic-apollo · 2022-08-13T15:16:28Z

Motd icon will break on zooming in after its been drawn, already mentioned in commit message, that's why default is back to static.

And not sure why motd icon is too big a deal to be kept, considering its issues. If people are gonna actually run any commands in the terminal (instead of it just being riced up for a photo op for instagram/reddit ;)), it's gonna disappear in scrollback history anyways.

TomJo2000 · 2022-08-13T21:57:32Z

If anyone zooms, the style can not be changed.

As of right now, that is correct.
Making it dynamically resize itself would add a lot of overhead because you'd need to detect zooming.
It's probably doable, but it's probably also not worth the effort.

Edit: That being said... Let me see if trap-ing SIGWINCH would even be able to detect zooming in Termux.
For all I know that signal might not even exist in Android since it isn't part of the standard POSIX set.

Edit 2: SIGWINCH is ignored by the shell by default, so trap would not work on it.

- The logo added in new dynamic motd would break due to word wrap if terminal columns were less than max line length. This fixes the issue by not drawing a logo if terminal columns are too few at draw time. Note that logo will still break if terminal size is changed after drawing. - Use TERMUX_APP_PACKAGE_MANAGER instead of TERMUX_MAIN_PACKAGE_FORMAT - Add donate link instead of gitter which should already exist in community link, including matrix rooms link. Related pull requests termux/termux-packages#11250 and termux/termux-packages#11294

…motd and add support for custom motd The dynamic motd has issues with logo word wrap as discussed in 5956876 and so can't be used as default. Users who want to use it or a custom one can create a motd file/symlink at `~/.termux/motd.sh` to load a custom motd. To use the dynamic motd provided by termux-tools package, run `ln -sf $PREFIX/etc/motd.sh ~/.termux/motd.sh` Also directly execute motd.sd and rely on interpreter instead of executing with bash in case its not available or user does not want a bash script as motd.sh

agnostic-apollo · 2022-08-15T09:53:02Z

Made a change to directly execute motd.sd and rely on interpreter instead of executing with bash in case its not available or user does not want a bash script as motd.sh.

Merging since no issues reported till now (that need fixing).

TomJo2000 reviewed Aug 13, 2022

View reviewed changes

truboxl reviewed Aug 13, 2022

View reviewed changes

motds/Makefile.am Show resolved Hide resolved

kfatyuip reviewed Aug 13, 2022

View reviewed changes

agnostic-apollo added 3 commits August 15, 2022 14:46

motds: move motds to separate directory

d17067d

agnostic-apollo force-pushed the motd-updates branch from 0fb528e to 6a0f5cc Compare August 15, 2022 09:49

agnostic-apollo merged commit 6a0f5cc into main Aug 15, 2022

agnostic-apollo deleted the motd-updates branch August 15, 2022 16:00

agnostic-apollo mentioned this pull request Aug 25, 2022

[Information] packages.termux.dev may be shutdown at some point termux/termux-packages#11705

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Motd updates #8

Motd updates #8

agnostic-apollo commented Aug 13, 2022

TomJo2000 commented Aug 13, 2022

TomJo2000 left a comment

TomJo2000 Aug 13, 2022

agnostic-apollo Aug 13, 2022

TomJo2000 Aug 13, 2022

TomJo2000 Aug 13, 2022

agnostic-apollo Aug 13, 2022 •

edited

Loading

TomJo2000 Aug 13, 2022

TomJo2000 Aug 13, 2022

agnostic-apollo Aug 13, 2022

TomJo2000 Aug 13, 2022

TomJo2000 Aug 13, 2022

TomJo2000 Aug 13, 2022

TomJo2000 Aug 13, 2022

agnostic-apollo Aug 13, 2022

TomJo2000 Aug 13, 2022 •

edited

Loading

TomJo2000 Aug 13, 2022

agnostic-apollo commented Aug 13, 2022 •

edited

Loading

kfatyuip left a comment

agnostic-apollo commented Aug 13, 2022

kfatyuip commented Aug 13, 2022

agnostic-apollo commented Aug 13, 2022

TomJo2000 commented Aug 13, 2022 •

edited

Loading

agnostic-apollo commented Aug 15, 2022

	if [ -f ~/.termux/motd.sh ] && [ -f @TERMUX_PREFIX@/bin/bash ] && [ -x @TERMUX_PREFIX@/bin/bash ]; then
	if [ -f ~/.termux/motd.sh ] && [ -x @TERMUX_PREFIX@/bin/bash ]; then


		else

		motd="

Motd updates #8

Motd updates #8

Conversation

agnostic-apollo commented Aug 13, 2022

TomJo2000 commented Aug 13, 2022

TomJo2000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agnostic-apollo Aug 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomJo2000 Aug 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agnostic-apollo commented Aug 13, 2022 • edited Loading

kfatyuip left a comment

Choose a reason for hiding this comment

agnostic-apollo commented Aug 13, 2022

kfatyuip commented Aug 13, 2022

agnostic-apollo commented Aug 13, 2022

TomJo2000 commented Aug 13, 2022 • edited Loading

agnostic-apollo commented Aug 15, 2022

agnostic-apollo Aug 13, 2022 •

edited

Loading

TomJo2000 Aug 13, 2022 •

edited

Loading

agnostic-apollo commented Aug 13, 2022 •

edited

Loading

TomJo2000 commented Aug 13, 2022 •

edited

Loading