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.

Sign up for GitHub

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

Jump to bottom

Add infobox #316

Merged
merged 9 commits into from
Jul 4, 2021
Merged

Add infobox #316

merged 9 commits into from
Jul 4, 2021

Conversation

pickfire
Copy link
Contributor

@pickfire pickfire commented Jun 19, 2021 •
edited
Loading

  • Render the info box
  • Render the table
  • Generate the doc (bad)
  • Generate rustdoc

Progress (goto and space mode done)

Screenshot_20210704_001405

image

Screenshot_20210704_001422

image

@pickfire pickfire force-pushed the infobox branch 5 times, most recently from 238502d to 2074a24 Compare June 26, 2021 17:07
pickfire
pickfire commented Jun 26, 2021
View reviewed changes
helix-term/src/ui/info.rs Outdated Show resolved Hide resolved
@pickfire pickfire force-pushed the infobox branch 3 times, most recently from 52d1797 to 37524f4 Compare June 28, 2021 01:20
@pickfire pickfire requested review from archseer, CBenoit and cessen June 28, 2021 01:21
@pickfire pickfire marked this pull request as ready for review June 28, 2021 01:22
pickfire
pickfire commented Jun 28, 2021
View reviewed changes
helix-term/src/commands.rs Outdated Show resolved Hide resolved
CBenoit
CBenoit reviewed Jun 28, 2021
View reviewed changes
Copy link
Member

@CBenoit CBenoit left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good job!
I had a quick look at the code, here are some suggestions.

helix-term/src/keymap.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
cessen
cessen reviewed Jun 29, 2021
View reviewed changes
helix-term/src/commands.rs Outdated Show resolved Hide resolved
pickfire
pickfire commented Jun 29, 2021
View reviewed changes
helix-term/src/commands.rs Outdated Show resolved Hide resolved
pickfire
pickfire commented Jun 29, 2021
View reviewed changes
helix-term/src/commands.rs Outdated Show resolved Hide resolved
pickfire
pickfire commented Jun 29, 2021
View reviewed changes
helix-term/src/keymap.rs Outdated Show resolved Hide resolved
pickfire
pickfire commented Jun 29, 2021
View reviewed changes
helix-term/src/keymap.rs Outdated Show resolved Hide resolved
pickfire
pickfire commented Jun 29, 2021
View reviewed changes
helix-term/src/keymap.rs Outdated Show resolved Hide resolved
archseer
archseer requested changes Jun 29, 2021
View reviewed changes
helix-term/src/ui/info.rs Outdated Show resolved Hide resolved
helix-view/Cargo.toml Outdated Show resolved Hide resolved
helix-view/src/input.rs Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
@pickfire pickfire force-pushed the infobox branch 4 times, most recently from 3bf6bdd to 58bf9b5 Compare July 1, 2021 01:20
@pickfire pickfire self-assigned this Jul 1, 2021
@pickfire pickfire added A-documentation Area: Documentation improvements A-helix-term Area: Helix term improvements C-enhancement Category: Improvements labels Jul 1, 2021
@pickfire pickfire force-pushed the infobox branch 2 times, most recently from 716d2de to 3ee7b1b Compare July 2, 2021 01:50
@pickfire pickfire requested a review from archseer July 2, 2021 01:51
@archseer
Copy link
Member

archseer commented Jul 2, 2021

@pickfire Can you do a quick rebase? The lint cleanup made some conflicts.

Also, are you planning to change other modes too? Or are we merging just space mode currently?

@pickfire
Copy link
Contributor Author

pickfire commented Jul 2, 2021 •
edited
Loading

Also, are you planning to change other modes too? Or are we merging just space mode currently?

Yes, but can merge this first. I just did a rebase this morning though.

@pickfire pickfire force-pushed the infobox branch 3 times, most recently from f6491ed to 7515053 Compare July 2, 2021 11:19
CBenoit
CBenoit approved these changes Jul 2, 2021
View reviewed changes
Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

Looks good to me

@archseer
Copy link
Member

archseer commented Jul 2, 2021

There's a failed lint & the TODO: generated, delete it later comment. Once those are fixed this can be merged.

pickfire and others added 7 commits July 3, 2021 20:36
@pickfire
Add infobox
7bad6cb
@pickfire @CBenoit
Suggestions for infobox changes
15760d2 
Co-authored-by: Benoît Cortier <benoit.cortier@fried-world.eu>
@pickfire
Apply suggestions from blaz for infobox
be56f0f
@pickfire
Fix rendering issues for infobox
fafed70
@pickfire
Add infobox doc generation and improve ergonomics
5302de6
@pickfire
Support infobox doc gen on stable release
78f1a3d
@pickfire
Reduce calculation and improve pattern in infobox
2a11b97 
- switch to use static OnceCell to calculate Info once
- pass Vec<(&[KeyEvent], &str)> rather than Vec<(Vec<KeyEvent>, &str)>
- expr -> tt to allow using | as separator, make it more like match
pickfire added 2 commits July 4, 2021 00:00
@pickfire
Goto mode use infobox
996f5cd 
In the meantime, change gm to gc.
Remove extra space in mode title.
@pickfire
Info box add horizontal padding
087f032
@archseer archseer merged commit 916362d into helix-editor:master Jul 4, 2021
@pickfire pickfire deleted the infobox branch July 4, 2021 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cessen cessen cessen left review comments

@CBenoit CBenoit CBenoit approved these changes

@archseer archseer Awaiting requested review from archseer

Assignees

@pickfire pickfire

Labels
A-documentation Area: Documentation improvements A-helix-term Area: Helix term improvements C-enhancement Category: Improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pickfire @archseer @cessen @CBenoit