Feature/help #74
Merged
Feature/help #74
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Gituser143
requested changes
Oct 8, 2020
There was a problem hiding this comment.
@anihm136 the deletions to updateUI()
A full screen render ends up looking like this. It only resizes during the tick.
break the existing resizing and rendering.
When switching back, it ends up looking like so (that is not a cropped image, that is the entire terminal :P):
We need the ui.Render(mypage.grid) back in the updateUI() function and the calls to updateUI() are necessary too.
EDIT: To recreate the issue, run grofer -r 3000 and resize the terminal.
|
Okay looks like the issue occurs in between ticks right. I did not test that very well. The reason I had to remove that is coz it re-renders the main page over the help page on resize. I could have moved the same logic I'm using on keypress there also, but it seemed to be working fine without (since I did not test at the slower refresh rates). I'll move the logic into the update function also |
8000
|
There might be a simpler fix for the re rendering of UI. You could call the pause function to stop the main UI rendering. And once the help dialog is closed, call the pause function again to resume rendering. Hopefully this might make your tasks easier to solve. |
|
@Gituser143 should be sorted now, can you check? |
|
@anihm136 3 things you missed out:
Additionally, maybe use |
Gituser143
reviewed
Oct 8, 2020
anihm136
commented
Oct 8, 2020
|
@Gituser143 First and second points should be fixed now. For the third point, as I mentioned in one of the comments, it does not update in the background because there's a condition checking if the help menu is visible and avoiding re-renders then |
|
@anihm136 There's an unnecessary empty line at the end main and proc_pid keybindings. It shows up as so when scrolled to the bottom. This can be removed. EDIT: Could you please suffix and prefix a space in the title? Just to be consistent with the others. It should look like |
|
@Gituser143 should be fixed now |
|
@anihm136 for some reason, the |
Gituser143
requested changes
Oct 9, 2020
Gituser143
requested changes
Oct 9, 2020
There was a problem hiding this comment.
These look good! @anihm136 There are 1 or 2 places where you've commented out code. Could you please remove them?
There was a problem hiding this comment.
Hi,
First off, this is great work @anihm136!
I had a query which I was hoping you could answer:
- Could
PROC_KEYBINDSbe made an array of strings, since that's what thelistwidget oftermuiaccepts, the reason I ask this is simply from an efficiency point of view, I noticed that you are using theSplit()function inResize(). Every time theResize()function is called,Split()will iterate over the string as well as allocate an extra array which is ultimately returned, although this is garbage collected, we could maybe try and improve that aspect, and the same logic applies to theCount()function as well (you could uselen(arrayName)). I'd like your opinion on this and whether you think it's feasible to implement without compromising the readability/simplicity of your code.
MadhavJivrajani
requested changes
Oct 9, 2020
There was a problem hiding this comment.
- Could you add info about the keybinding to the README?
- Now that Press F to kill #54 is merged, maybe add that keybinding for
grofer procas well (Kor<F9>to kill a process).
I think the main reason I used the string itself is for readability of the formatting. In the keybindings for proc you can see that there is some indentation, which might be confusing if implemented as an array of strings. If that's not an issue I can change it to an array of strings As for the docs and the kill keybinding, I'll add it |
Hmm, I understand, my recommendation would still be to convert it to an array of strings since that would be pre-allocated by the compiler and would not grow/shrink at run time and any overheads of garbage collection wrt to that |
ui.List provides some styling and a simpler way to load data, as well as option to wrap lines, allowing easier resizing of the widget
Escape was mapped to exiting the help screen, but the help menu would persist for a while on top of the page (until the next tick). This causes the redraw to happen immediately
Add scroll option for the help widget when all lines are not visible. Also fix the render delay when help widget is exited with <Esc>
|
Okay, that was some insane merge conflict. @Gituser143 the commented code should be removed now, let me know if I've missed someplace. @MadhavJivrajani I've changed it to use a slice of strings instead of a string, and the new keybinding has been added as well. Only documentation is remaining |
Gituser143
approved these changes
Oct 9, 2020
There was a problem hiding this comment.
@anihm136 This looks good to me! I might have missed something though. @MadhavJivrajani @metonymic-smokey plis review.
MadhavJivrajani
added a commit
that referenced
this pull request
Jan 3, 2021
* Implement JSON per proc export * Update switch conditions for export * Restructured packages * Add license to procExport.go * Fixed package name, removed output file before creation * issue #65 (#68) * issue #65 * fix: uint32->int32 * fix wrong type in flag initialization in root.go * Add keybindings to kill process in list (#16) Avoid storing all procs, parse PID when killing the process * Add error handling in process kill Exit proc gracefully and fix formatting * Change Kill key to K, ignore error if PID does not exist * confirm process kill, refactor some parts * Change color of selected process when killing Paint row red on error in proc Also, changed row color to magenta when selected for killing * Check if process still exists when killing - renamed `paused` to `killSelected` - removed unnecessary once statement in updateProcs - if a process is selected for killing, check if the PID still exists on every update Fixed selection color not persisting * Fix UI closing in proc -p * Feature/help (#74) * Toggle help window in procs page * Refactor to separate help message from widget * Use ui.List instead of ui.Block ui.List provides some styling and a simpler way to load data, as well as option to wrap lines, allowing easier resizing of the widget * Fix resizing of help widget * Add keybindings for main and `proc -pid` pages * Add help widget to `proc -pid` page * Add help widget to main page * Fix escape keybinding to clear screen first Escape was mapped to exiting the help screen, but the help menu would persist for a while on top of the page (until the next tick). This causes the redraw to happen immediately * Fix resizing issue * Fix formatting error * Add scroll in help menu, fix render delay Add scroll option for the help widget when all lines are not visible. Also fix the render delay when help widget is exited with <Esc> * Fix formatting and spacing issues * Fix procGraphs rendering * Remove unneccesary comment * Use slice of strings instead of string * Add kill keybind to help message * Remove unused module * Re-add scroll in help menu * Fix formatting issue * update readme and root.go for documentation * update readme for feature/help * Fix UI flickering. Removed unnecessary calls to ui.Clear() * Added cute beaver and elephant ASCII Art * Clear background UI on rendering help * Fix UI clearing for grofer --cpudinfo and initialize help * made functions not exportable, fixed error in random selection# * Removed comments of unexported Functions * GH-75: using after channel instead of delay so exit is immediate (#77) * GH-75: using after channel instead of delay so exit is immediate * GH-75 BUG exit time too slow - Using single select loop for generalStats. - Using select time.After instead of time.Delay for proc single and multiple. * GH-75: using ticker instead of after * GH-75: using common tick util for cpu info * GH-75: added license to tickutils.go * Made general export line by line json * Add export functionality for per proc * Add .circleci/config.yml * Set working directory for circleci * Modified tests * Attempt dependecy fix for test * Update go version * Attempt python install * Attempt python install as root * Remove travis and testing circleci * Fix indentation and shift CI to circleci * Add .circleci/config.yml * Remove extra comments from CI config * Removed redundant export files * Fix formatting * Update license in cpuInfo * Fix precision for exported fields * Add prompt to confirm profile overwrites * Moved roundOff to utils and removed redundant print statements * Add overwrite prompt to per proc export, format prompt text with filname * Log errors in export iterations * Nested error handling * Update documentation * Remove mentions of JSON * Add export type explanation Co-authored-by: Prince Verma <prncvrm@gmail.com> Co-authored-by: Madhav Jivrajani <madhav.jiv@gmail.com> Co-authored-by: Samyak S Sarnayak <samyak201@gmail.com> Co-authored-by: Anirudh Haritas Murali <49116134+anihm136@users.noreply.github.com> Co-authored-by: localleon <me@lrau.xyz> Co-authored-by: ataboo <doubar2001@gmail.com>
MadhavJivrajani
added a commit
that referenced
this pull request
Jan 3, 2021
* added boilerplate code for exporting json * forgot license :p * removed test json * fixed file descriptors * add export functionality for json * added better validation of file names and slight refactoring done * remove explicit extension append for default case * ocd fix * delete output of test run * remove unescessary comments * change export type validation in map * change exportable consts to non-exportable consts * change export type validation * Add JSON export functionality (#84) * Implement JSON per proc export * Update switch conditions for export * Restructured packages * Add license to procExport.go * Fixed package name, removed output file before creation * issue #65 (#68) * issue #65 * fix: uint32->int32 * fix wrong type in flag initialization in root.go * Add keybindings to kill process in list (#16) Avoid storing all procs, parse PID when killing the process * Add error handling in process kill Exit proc gracefully and fix formatting * Change Kill key to K, ignore error if PID does not exist * confirm process kill, refactor some parts * Change color of selected process when killing Paint row red on error in proc Also, changed row color to magenta when selected for killing * Check if process still exists when killing - renamed `paused` to `killSelected` - removed unnecessary once statement in updateProcs - if a process is selected for killing, check if the PID still exists on every update Fixed selection color not persisting * Fix UI closing in proc -p * Feature/help (#74) * Toggle help window in procs page * Refactor to separate help message from widget * Use ui.List instead of ui.Block ui.List provides some styling and a simpler way to load data, as well as option to wrap lines, allowing easier resizing of the widget * Fix resizing of help widget * Add keybindings for main and `proc -pid` pages * Add help widget to `proc -pid` page * Add help widget to main page * Fix escape keybinding to clear screen first Escape was mapped to exiting the help screen, but the help menu would persist for a while on top of the page (until the next tick). This causes the redraw to happen immediately * Fix resizing issue * Fix formatting error * Add scroll in help menu, fix render delay Add scroll option for the help widget when all lines are not visible. Also fix the render delay when help widget is exited with <Esc> * Fix formatting and spacing issues * Fix procGraphs rendering * Remove unneccesary comment * Use slice of strings instead of string * Add kill keybind to help message * Remove unused module * Re-add scroll in help menu * Fix formatting issue * update readme and root.go for documentation * update readme for feature/help * Fix UI flickering. Removed unnecessary calls to ui.Clear() * Added cute beaver and elephant ASCII Art * Clear background UI on rendering help * Fix UI clearing for grofer --cpudinfo and initialize help * made functions not exportable, fixed error in random selection# * Removed comments of unexported Functions * GH-75: using after channel instead of delay so exit is immediate (#77) * GH-75: using after channel instead of delay so exit is immediate * GH-75 BUG exit time too slow - Using single select loop for generalStats. - Using select time.After instead of time.Delay for proc single and multiple. * GH-75: using ticker instead of after * GH-75: using common tick util for cpu info * GH-75: added license to tickutils.go * Made general export line by line json * Add export functionality for per proc * Add .circleci/config.yml * Set working directory for circleci * Modified tests * Attempt dependecy fix for test * Update go version * Attempt python install * Attempt python install as root * Remove travis and testing circleci * Fix indentation and shift CI to circleci * Add .circleci/config.yml * Remove extra comments from CI config * Removed redundant export files * Fix formatting * Update license in cpuInfo * Fix precision for exported fields * Add prompt to confirm profile overwrites * Moved roundOff to utils and removed redundant print statements * Add overwrite prompt to per proc export, format prompt text with filname * Log errors in export iterations * Nested error handling * Update documentation * Remove mentions of JSON * Add export type explanation Co-authored-by: Prince Verma <prncvrm@gmail.com> Co-authored-by: Madhav Jivrajani <madhav.jiv@gmail.com> Co-authored-by: Samyak S Sarnayak <samyak201@gmail.com> Co-authored-by: Anirudh Haritas Murali <49116134+anihm136@users.noreply.github.com> Co-authored-by: localleon <me@lrau.xyz> Co-authored-by: ataboo <doubar2001@gmail.com> Co-authored-by: Bhargav SNV <44526455+Gituser143@users.noreply.github.com> Co-authored-by: Prince Verma <prncvrm@gmail.com> Co-authored-by: Samyak S Sarnayak <samyak201@gmail.com> Co-authored-by: Anirudh Haritas Murali <49116134+anihm136@users.noreply.github.com> Co-authored-by: localleon <me@lrau.xyz> Co-authored-by: ataboo <doubar2001@gmail.com>
MadhavJivrajani
added a commit
that referenced
this pull request
Jan 4, 2021
* Add feature for more CPU info (#46) * added code for additional cpu info * added cpu rates to CPULoad type * Made UI for CPU info * Cleaned use fo sync.once variables for faster UI rendering Co-authored-by: Madhav Jivrajani <madhav.jiv@gmail.com> * added cpuinfo in README * removed stray comment * Update README * Add extra comments * Refactored printStats, becoming serveStats! (#50) Co-authored-by: lrb <lrb@envs.net> * Readme update * Remove duplicate error printing. (#52) * added common error general.ErrCanceledByUser * Fix error handlings under cmd/root (#48) * use context to cancel executions * use errgroup in cmd/root.go * every functions returns error * use context to stop ServerFuncs * add license to src/general/errors.go * Add completion subcommand * Clean up description and remove generic stuff * Added license header * Render Grid on scroll to fix UI lag (#60) * Replaced for loops with strings.Repeat() for whitespace generation * inconsistent allignment in list of child process for gropher proc command (#61) * Fixes #45 (inconsistent allignment in list of child process for command) * remove temp test logic and run gofmt * add yellow color to PID text irrespective of any condition * make color scheme consistent with AllProcsPage * Add Cute ASCII for error messages! (#62) * Added cute ascii art! * Updated goobe art * Renamed doggo.go to error.go * Change tabs to spaces in license * Change yourprogram to grofer * Add section about completion in readme * refactored flag variable default values into consts (#67) * refactored flag variable default values into consts * removed comment and added new line * checking with default value before validation * Fixed issues with text being invisible under bright backgrounds termui defaults to ColorWhite, which displays white text irrespective of terminal. Using ColorClear instead uses the default text color that the terminal uses. ISSUES: Plot widget does not accept changes to the color of the axes for some reason, hence it remains grey under both Dark and Light-colored backgrounds. * issue #65 (#68) * issue #65 * fix: uint32->int32 * fix wrong type in flag initialization in root.go * Add keybindings to kill process in list (#16) Avoid storing all procs, parse PID when killing the process * Add error handling in process kill Exit proc gracefully and fix formatting * Change Kill key to K, ignore error if PID does not exist * confirm process kill, refactor some parts * Change color of selected process when killing Paint row red on error in proc Also, changed row color to magenta when selected for killing * Check if process still exists when killing - renamed `paused` to `killSelected` - removed unnecessary once statement in updateProcs - if a process is selected for killing, check if the PID still exists on every update Fixed selection color not persisting * Fix UI closing in proc -p * Feature/help (#74) * Toggle help window in procs page * Refactor to separate help message from widget * Use ui.List instead of ui.Block ui.List provides some styling and a simpler way to load data, as well as option to wrap lines, allowing easier resizing of the widget * Fix resizing of help widget * Add keybindings for main and `proc -pid` pages * Add help widget to `proc -pid` page * Add help widget to main page * Fix escape keybinding to clear screen first Escape was mapped to exiting the help screen, but the help menu would persist for a while on top of the page (until the next tick). This causes the redraw to happen immediately * Fix resizing issue * Fix formatting error * Add scroll in help menu, fix render delay Add scroll option for the help widget when all lines are not visible. Also fix the render delay when help widget is exited with <Esc> * Fix formatting and spacing issues * Fix procGraphs rendering * Remove unneccesary comment * Use slice of strings instead of string * Add kill keybind to help message * Remove unused module * Re-add scroll in help menu * Fix formatting issue * update readme and root.go for documentation * update readme for feature/help * Fix UI flickering. Removed unnecessary calls to ui.Clear() * Added cute beaver and elephant ASCII Art * Clear background UI on rendering help * Fix UI clearing for grofer --cpudinfo and initialize help * made functions not exportable, fixed error in random selection# * Removed comments of unexported Functions * GH-75: using after channel instead of delay so exit is immediate (#77) * GH-75: using after channel instead of delay so exit is immediate * GH-75 BUG exit time too slow - Using single select loop for generalStats. - Using select time.After instead of time.Delay for proc single and multiple. * GH-75: using ticker instead of after * GH-75: using common tick util for cpu info * GH-75: added license to tickutils.go * Add .circleci/config.yml * Set working directory for circleci * Modified tests * Attempt dependecy fix for test * Update go version * Attempt python install * Attempt python install as root * Remove travis and testing circleci * Fix indentation and shift CI to circleci * Add .circleci/config.yml * Remove extra comments from CI config * Show correct error msg. on neg. pids (#86) Giving a pid 0 from `grofer proc -p 0` is the same as `grofer proc`. Fixes #86 * Documented PID 0 in grofer proc * Add export functionality (#88) * added boilerplate code for exporting json * forgot license :p * removed test json * fixed file descriptors * add export functionality for json * added better validation of file names and slight refactoring done * remove explicit extension append for default case * ocd fix * delete output of test run * remove unescessary comments * A1DD change export type validation in map * change exportable consts to non-exportable consts * change export type validation * Add JSON export functionality (#84) * Implement JSON per proc export * Update switch conditions for export * Restructured packages * Add license to procExport.go * Fixed package name, removed output file before creation * issue #65 (#68) * issue #65 * fix: uint32->int32 * fix wrong type in flag initialization in root.go * Add keybindings to kill process in list (#16) Avoid storing all procs, parse PID when killing the process * Add error handling in process kill Exit proc gracefully and fix formatting * Change Kill key to K, ignore error if PID does not exist * confirm process kill, refactor some parts * Change color of selected process when killing Paint row red on error in proc Also, changed row color to magenta when selected for killing * Check if process still exists when killing - renamed `paused` to `killSelected` - removed unnecessary once statement in updateProcs - if a process is selected for killing, check if the PID still exists on every update Fixed selection color not persisting * Fix UI closing in proc -p * Feature/help (#74) * Toggle help window in procs page * Refactor to separate help message from widget * Use ui.List instead of ui.Block ui.List provides some styling and a simpler way to load data, as well as option to wrap lines, allowing easier resizing of the widget * Fix resizing of help widget * Add keybindings for main and `proc -pid` pages * Add help widget to `proc -pid` page * Add help widget to main page * Fix escape keybinding to clear screen first Escape was mapped to exiting the help screen, but the help menu would persist for a while on top of the page (until the next tick). This causes the redraw to happen immediately * Fix resizing issue * Fix formatting error * Add scroll in help menu, fix render delay Add scroll option for the help widget when all lines are not visible. Also fix the render delay when help widget is exited with <Esc> * Fix formatting and spacing issues * Fix procGraphs rendering * Remove unneccesary comment * Use slice of strings instead of string * Add kill keybind to help message * Remove unused module * Re-add scroll in help menu * Fix formatting issue * update readme and root.go for documentation * update readme for feature/help * Fix UI flickering. Removed unnecessary calls to ui.Clear() * Added cute beaver and elephant ASCII Art * Clear background UI on rendering help * Fix UI clearing for grofer --cpudinfo and initialize help * made functions not exportable, fixed error in random selection# * Removed comments of unexported Functions * GH-75: using after channel instead of delay so exit is immediate (#77) * GH-75: using after channel instead of delay so exit is immediate * GH-75 BUG exit time too slow - Using single select loop for generalStats. - Using select time.After instead of time.Delay for proc single and multiple. * GH-75: using ticker instead of after * GH-75: using common tick util for cpu info * GH-75: added license to tickutils.go * Made general export line by line json * Add export functionality for per proc * Add .circleci/config.yml * Set working directory for circleci * Modified tests * Attempt dependecy fix for test * Update go version * Attempt python install * Attempt python install as root * Remove travis and testing circleci * Fix indentation and shift CI to circleci * Add .circleci/config.yml * Remove extra comments from CI config * Removed redundant export files * Fix formatting * Update license in cpuInfo * Fix precision for exported fields * Add prompt to confirm profile overwrites * Moved roundOff to utils and removed redundant print statements * Add overwrite prompt to per proc export, format prompt text with filname * Log errors in export iterations * Nested error handling * Update documentation * Remove mentions of JSON * Add export type explanation Co-authored-by: Prince Verma <prncvrm@gmail.com> Co-authored-by: Madhav Jivrajani <madhav.jiv@gmail.com> Co-authored-by: Samyak S Sarnayak <samyak201@gmail.com> Co-authored-by: Anirudh Haritas Murali <49116134+anihm136@users.noreply.github.com> Co-authored-by: localleon <me@lrau.xyz> Co-authored-by: ataboo <doubar2001@gmail.com> Co-authored-by: Bhargav SNV <44526455+Gituser143@users.noreply.github.com> Co-authored-by: Prince Verma <prncvrm@gmail.com> Co-authored-by: Samyak S Sarnayak <samyak201@gmail.com> Co-authored-by: Anirudh Haritas Murali <49116134+anihm136@users.noreply.github.com> Co-authored-by: localleon <me@lrau.xyz> Co-authored-by: ataboo <doubar2001@gmail.com> * removed extraneous RenderCPUInfo func Co-authored-by: Bhargav SNV <44526455+Gituser143@users.noreply.github.com> Co-authored-by: Gituser143 <bhargavsnv100@gmail.com> Co-authored-by: mattharwood <matt@mattharwood.com> Co-authored-by: lrb <lrb@envs.net> Co-authored-by: __touk__ <zerouali.t@gmail.com> Co-authored-by: egawata <egawa.takashi@gmail.com> Co-authored-by: Anirudh H M <anihm136@gmail.com> Co-authored-by: Siddhant Sinha <siddhant94@users.noreply.github.com> Co-authored-by: Anirudh Haritas Murali <49116134+anihm136@users.noreply.github.com> Co-authored-by: Souvik Maji <souvikmaji94@gmail.com> Co-authored-by: Kunal Bhat <kunal.bhat2001@gmail.com> Co-authored-by: Prince Verma <prncvrm@gmail.com> Co-authored-by: Samyak S Sarnayak <samyak201@gmail.com> Co-authored-by: localleon <me@lrau.xyz> Co-authored-by: ataboo <doubar2001@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds a modular help widget, which can be used to add a help screen to any page with appropriate help string. Also adds help widget to existing pages
Fixes #42
Type of change
Please delete options that are not relevant.
Checklist:
go fmton my code (reference)