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

Polish welcome message #273

Merged
merged 7 commits into from
Mar 31, 2025
Merged

Polish welcome message #273

merged 7 commits into from
Mar 31, 2025

Conversation

SimplyDanny
Copy link
Contributor

@SimplyDanny SimplyDanny commented Mar 29, 2025

This applies some polishing on the textual output of the command line tool, namely:

  • Inline the hash -r advice.
  • Mention the location where toolchains go.
  • Improve text a little generally.
  • Restructure the code to built up the message.

Copy link
Member

@cmcgee1024 cmcgee1024 left a comment

Choose a reason for hiding this comment

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

praise: Thank you for making the intro screen much clearer and cleaner.

I just have one suggestion, and then I think this is good to be merged.

Copy link
Member

@heckj heckj left a comment

Choose a reason for hiding this comment

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

Good updates - I'd suggest a minor additional tweak in one place


hash -r
NOTE: Swiftly has updated some elements in your path and your shell may not yet be
aware of the changes. You can run 'hash -r' to update your shell in place.
Copy link
Member

Choose a reason for hiding this comment

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

question: Can the command go onto its own line here? While the indentation is not very consistent at the moment, the idea is to have each command the user is being asked to run on separate lines so that they can select, copy, paste, and run up them more easily than trying to select from inside a paragraph.

Copy link
Contributor Author

@SimplyDanny SimplyDanny Mar 30, 2025

Choose a reason for hiding this comment

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

I've made it so that the command is on a separate line easy to copy and added the info that restarting the shell is an option, too. I personally like it when the reading flow isn't interrupted by colons only to name things.

the integrity of the downloads.

"""
#endif
Copy link
Member

Choose a reason for hiding this comment

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

question: Since you're making this summary much better than what was there before can you also put in a section that describes the changes to the user's profile? This was something being asked in #255 . This summary is trying to gain user trust through transparency, so that they don't just hit n / ctrl-c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's done now. The logic is as before: When suppressing commands are not given, the text is printed to let the user know about this significant change. It now also mentions the command line option that'd suppress it right away.

@SimplyDanny
Copy link
Contributor Author

I've added a short, perhaps a little kitschy, introduction as well. Let me know what you think about it.

@SimplyDanny
Copy link
Contributor Author

The more I added and customized the output, the more convinced I became that swiftly would benefit from adopting Noora. It was made for exactly the use case of guiding users through steps and processes on the command line. Just in case you didn't know about Noora as of yet ... 😉

@cmcgee1024
Copy link
Member

@SimplyDanny this looks really good. It's currently failing the formatting check. Can you run the following to fix the formatting, and update the PR?

swift run swiftformat .

@SimplyDanny
Copy link
Contributor Author

@SimplyDanny this looks really good. It's currently failing the formatting check. Can you run the following to fix the formatting, and update the PR?

swift run swiftformat .

That's done.

@cmcgee1024
Copy link
Member

@swift-ci test macOS

@cmcgee1024
Copy link
Member

Thank you @SimplyDanny for your contribution.

@cmcgee1024 cmcgee1024 merged commit fd32a53 into swiftlang:main Mar 31, 2025
19 checks passed
@SimplyDanny SimplyDanny deleted the patch-1 branch March 31, 2025 16:36
cmcgee1024 pushed a commit to cmcgee1024/swiftly that referenced this pull request Mar 31, 2025
* List toolchain location and restructure message concatenation

* Add introduction

* Mention modification of profile
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.

3 participants