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

Reduce crate size using cargo diet by soon to be 26MB #29

Merged
merged 2 commits into from Apr 25, 2021

Conversation

Byron
Copy link
Contributor

@Byron Byron commented Apr 25, 2021

Here is the output it produced.

hackernews-TUI git:(main) cargo diet
┌───────────────────────────────────────┬─────────────┐
│ Removed File                          │ Size (Byte) │
├───────────────────────────────────────┼─────────────┤
│ run_debug                             │          93 │
│ .gitignore                            │         177 │
│ examples/hn-tui.toml                  │        2056 │
│ examples/assets/story_search_view.png │      153376 │
│ examples/assets/comment_view.png      │      160630 │
│ examples/assets/story_view.png        │      216828 │
│ examples/assets/help_view.png         │      293248 │
│ examples/assets/v0-5-demo-2.gif       │     9951136 │
│ examples/assets/v0-5-demo-1.gif       │    15331851 │
└───────────────────────────────────────┴─────────────┘
Saved 100% or 26.1 MB in 9 files (of 26.2 MB and 28 files in entire crate)

Please note that I found it easiest to move the default configuration
file that is included in main.rs into the root, similar to how
alacritty does it. There it might be more discoverable, too.

Also, please let me know if there is anything missing to make this
mergeable.

PS: Congrats to making it to the front page!

Here is the output it produced.

```
hackernews-TUI git:(main) cargo diet
┌───────────────────────────────────────┬─────────────┐
│ Removed File                          │ Size (Byte) │
├───────────────────────────────────────┼─────────────┤
│ run_debug                             │          93 │
│ .gitignore                            │         177 │
│ examples/hn-tui.toml                  │        2056 │
│ examples/assets/story_search_view.png │      153376 │
│ examples/assets/comment_view.png      │      160630 │
│ examples/assets/story_view.png        │      216828 │
│ examples/assets/help_view.png         │      293248 │
│ examples/assets/v0-5-demo-2.gif       │     9951136 │
│ examples/assets/v0-5-demo-1.gif       │    15331851 │
└───────────────────────────────────────┴─────────────┘
Saved 100% or 26.1 MB in 9 files (of 26.2 MB and 28 files in entire crate)
```

Please note that I found it easiest to move the default configuration
file that is included in main.rs into the root, similar to how
alacritty does it. There it might be more discoverable, too.

Also, please let me know if there is anything missing to make this
mergeable.

PS: Congrats to making it to the front page!
@aome510
Copy link
Owner

aome510 commented Apr 25, 2021

Interesting! I don't know there is cargo diet to do thing like this.

This PR looks good to me. I'll add 1 more commit to change my debug script.

PS: Congrats to making it to the front page!

Thank you! Hope you enjoy using the application.

Edit: I just realize that this PR is from a fork. If possible, can you change the debug script in https://github.com/aome510/hackernews-TUI/blob/main/run_debug as well?

@Byron
Copy link
Contributor Author

Byron commented Apr 25, 2021

Interesting, you should be able to push directly into the PR. Here is how I usually do it using this PR as an example:

In this repository, run gh pr checkout 29, make the edits, and finish with git push. This will push directly into this PR. Alternatively you can merge the branch locally and push to main to close this PR automatically.

Just let me know if that's nothing you feel to do right now and I will make the changes as requested.

@aome510
Copy link
Owner

aome510 commented Apr 25, 2021

In this repository, run gh pr checkout 29, make the edits, and finish with git push. This will push directly into this PR.

Oh I see, I never use github-cli before.

Just let me know if that's nothing you feel to do right now and I will make the changes as requested.

Yeah I can do it! Thank you for the PR

@aome510 aome510 merged commit 37a1940 into aome510:main Apr 25, 2021
@Byron
Copy link
Contributor Author

Byron commented Apr 25, 2021

And thanks for the application, it definitely hits a sweet spot! I constantly want to hit ? while I memorize the shortcuts, but besides that it's working nicely out of the box.

Oh I see, I never use github-cli before.

That one definitely changed my life for the better especially when reviewing PRs and of course, creating them. Zero friction leads to me doing these cargo diet PRs whenever I come across a candidate crate.

@aome510
Copy link
Owner

aome510 commented Apr 25, 2021

And thanks for the application, it definitely hits a sweet spot! I constantly want to hit ? while I memorize the shortcuts, but besides that it's working nicely out of the box.

Yeah I plan to add ? in the near future. It seems that a lot assumes ? is the help key definitely not <ctrl-h> 😅

That one definitely changed my life for the better especially when reviewing PRs and of course, creating them. Zero friction leads to me doing these cargo diet PRs whenever I come across a candidate crate.

Cool, I'll definitely try it out and integrate into my workflow!

@Byron
Copy link
Contributor Author

Byron commented Apr 25, 2021

Yeah I plan to add ? in the near future. It seems that a lot assumes ? is the help key definitely not 😅

Haha, so there is a hotkey after all! That works too, but it doesn't seem to be listed in the menu :D. Maybe this can be an immediate fix of sorts. I did manage to click the help button and tried to go from there and assume many people get that far but will want to open the help regularly in their first sessions.

@aome510
Copy link
Owner

aome510 commented Apr 25, 2021

Ah! I guess that's because I assume that people opening the help dialog will use <ctrl-h> not the [help] button in the footer.

@Byron
Copy link
Contributor Author

Byron commented Apr 25, 2021

That's fair, I just wonder if it's only me who can't find a reference of ctrl-h anywhere, I seem to have no way of knowing.

@aome510
Copy link
Owner

aome510 commented Apr 25, 2021

That's fair, I just wonder if it's only me who can't find a reference of ctrl-h anywhere, I seem to have no way of knowing.

It is mentioned in the shortcut and global key shortcuts sections of the project description.

@Byron
Copy link
Contributor Author

Byron commented Apr 25, 2021

Ah, I think the difference is that I only launched the application and expected it to guide me - it did with a help button it's just that I couldn't figure out how to get there without using the mouse.

Screenshot 2021-04-25 at 17 47 35

This is one of my tools and I thought it would be best to use the available otherwise empty screenspace to help first-time users (or returning ones) along.

Screenshot 2021-04-25 at 18 40 16

I am saying this only to be sure I am not missing something and if not to show an opportunity for improving the onboarding experience of maybe just a few users (like me 😅).

@aome510
Copy link
Owner

aome510 commented Apr 25, 2021

Ah, I think the difference is that I only launched the application and expected it to guide me - it did with a help button it's just that I couldn't figure out how to get there without using the mouse.

Screenshot 2021-04-25 at 17 47 35

This is one of my tools and I thought it would be best to use the available otherwise empty screenspace to help first-time users (or returning ones) along.

Screenshot 2021-04-25 at 18 40 16

I am saying this only to be sure I am not missing something and if not to show an opportunity for improving the onboarding experience of maybe just a few users (like me sweat_smile).

yeah, you're right. I added <ctrl-h> to the help dialog recently, but I guess it would be better if I replace the [help] button in the footer with something like [help: ?/<ctrl-h>]. So first-time users can know the hotkey right away.

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.

None yet

2 participants