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

enhance(main/termux-tools): new motd design #11222

Closed
wants to merge 0 commits into from
Closed

enhance(main/termux-tools): new motd design #11222

wants to merge 0 commits into from

Conversation

TermuxMonet
Copy link
Contributor

@TermuxMonet TermuxMonet commented Jul 11, 2022

enhance(main/termux-tools): Added a new motd design

For aesthetic pruposes, changed the motd by adding a small termux icon, termux version display and a hint to fix repo issues related to termux-change repo.
And also, now login executes a dynamic motd.sh, which can expand environment variables and display text formatting.
Common motd support wasn't removed, since now the login just distinguishes between dynamic and common motd (prioritising the dynamic motd)

Preview:
Screenshot_20220710-214038_Termux~01

Co-authored-by: TomJo2000

Copy link
Member

@2096779623 2096779623 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TermuxMonet TermuxMonet requested a review from 2096779623 July 11, 2022 01:15
@TermuxMonet TermuxMonet changed the title New motd design enhance(main/termux-tools): new motd design Jul 11, 2022
Copy link
Member

@2096779623 2096779623 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please squash the commits.

@2096779623
Copy link
Member

2096779623 commented Jul 11, 2022

After pkg update, it will check whether the package can be upgraded. So change pkg update&&pkg upgrade to pkg update.

@TermuxMonet TermuxMonet requested a review from 2096779623 July 11, 2022 01:49
@TermuxMonet
Copy link
Contributor Author

After pkg update, it will check whether the package can be upgraded. So change pkg update&&pkg upgrade to pkg update.

Done!

@TermuxMonet
Copy link
Contributor Author

Oh wait, i'm gonna fix this unbound variable error, it should successfully build now

@TermuxMonet TermuxMonet requested a review from 2096779623 July 11, 2022 02:16
@agnostic-apollo
Copy link
Member

The icon you are adding on the left will break when text wrap engages when terminal width is less than characters per line.

@agnostic-apollo
Copy link
Member

And $TERMUX_VERSION will expand during build time, not on target device.

@agnostic-apollo
Copy link
Member

Also the motd file should just be text and normally not contain escape sequences since loading terminal may not support them, so it may break for some users. Probably not advisable to add this by default. Other members can give their opinion.

https://unix.stackexchange.com/questions/276185/how-do-i-create-a-dark-blue-text-on-a-bright-white-background-in-motd-in-debian

@TermuxMonet
Copy link
Contributor Author

TermuxMonet commented Jul 11, 2022

Also the motd file should just be text and normally not contain escape sequences since loading terminal may not support them, so it may break for some users. Probably not advisable to add this by default. Other members can give their opinion.

https://unix.stackexchange.com/questions/276185/how-do-i-create-a-dark-blue-text-on-a-bright-white-background-in-motd-in-debian

Okay, now i've replaced the unicode characters with white colored spaces, and shortened the lines for supporting the default zoom on termux

and replaced $TERMUX_VERSION with $TERMUX_APP__VERSION_NAME

@TermuxMonet TermuxMonet requested a review from 2096779623 July 11, 2022 03:53
@agnostic-apollo
Copy link
Member

and replaced $TERMUX_VERSION with $TERMUX_APP__VERSION_NAME

login script uses cat $PREFIX/etc/motd to load motd, so no variables are going to expand. The motd is not a script and is just a text/data file.

cat @TERMUX_PREFIX@/etc/motd

@TermuxMonet
Copy link
Contributor Author

and replaced $TERMUX_VERSION with $TERMUX_APP__VERSION_NAME

login script uses cat $PREFIX/etc/motd to load motd, so no variables are going to expand. The motd is not a script and is just a text/data file.

cat @TERMUX_PREFIX@/etc/motd

So that means, all lines with "echo -e" wouldn't be executed, and colored spaces, bold and underline wouldn't work

@agnostic-apollo
Copy link
Member

So that means, all lines with "echo -e" wouldn't be executed, and colored spaces, bold and underline wouldn't work

There will no echo -e lines in the motd file created because you are just echoing the escape sequences+text into the file as data. They will render as desired. You are supposed to check output files and test changes when sending pull requests.

@TermuxMonet
Copy link
Contributor Author

@TomJo2000 Provided a way to execute environment variables and colors in motd using ESC characters.
I'll reopen the pull request and fix the motd

@TermuxMonet TermuxMonet reopened this Jul 11, 2022
@TermuxMonet
Copy link
Contributor Author

Change of plans, i'll add a dynamic motd

@TomJo2000
Copy link
Member

Thought I'd drop in here for a quick comment.
We're still looking into how to do this in the least disruptive way possible.

The last thing I'd want is to break something with this.

@2096779623 2096779623 marked this pull request as draft July 11, 2022 15:44
@TermuxMonet TermuxMonet marked this pull request as ready for review July 11, 2022 16:49
@TermuxMonet
Copy link
Contributor Author

Now it's ready for building

packages/termux-tools/build.sh Outdated Show resolved Hide resolved
packages/termux-tools/build.sh Outdated Show resolved Hide resolved
@TermuxMonet TermuxMonet requested a review from Grimler91 July 11, 2022 20:04
@TermuxMonet
Copy link
Contributor Author

So.. should i replace wiki with IRC?

@agnostic-apollo
Copy link
Member

The community link already has IRC, so should be fine to remove it.

And instead of wiki, use Docs: https://termux.dev/docs. I will add wiki link there for now, wiki site will not be used in future.

@TermuxMonet
Copy link
Contributor Author

The community link already has IRC, so should be fine to remove it.

And instead of wiki, use Docs: https://termux.dev/docs. I will add wiki link there for now, wiki site will not be used in future.

Done

@TermuxMonet
Copy link
Contributor Author

TermuxMonet commented Jul 14, 2022

PR's TERMUX_PKG_VERSION Version is already being used by another approved PR now
And i can't change it because GitHub is gonna show conflict errors

@Grimler91
Copy link
Member

Version needs to be bumped, so please set it to 1.21 and force push to the branch

@TermuxMonet
Copy link
Contributor Author

Version needs to be bumped, so please set it to 1.21 and force push to the branch

Okay, but the PR will show some non related commits

@Grimler91
Copy link
Member

Okay, but the PR will show some non related commits

Please rebase and resolve conflicts to get rid of them

@TermuxMonet
Copy link
Contributor Author

Okay, but the PR will show some non related commits

Please rebase and resolve conflicts to get rid of them

They won't disappear, and the commit related to conflict fix won't appear in git rebase, even after git pull

@TermuxMonet
Copy link
Contributor Author

TermuxMonet commented Jul 14, 2022

Okay, but the PR will show some non related commits

Please rebase and resolve conflicts to get rid of them

Please help, the PR is broken now
i can't squash the commits or anything

@TermuxMonet TermuxMonet reopened this Jul 14, 2022
2096779623 pushed a commit that referenced this pull request Jul 19, 2022
Added a new termux motd dynamic design.
Closes #11222

Co-authored-by: TomJo2000 <tomjo00@web.de>
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.

5 participants