-
Notifications
You must be signed in to change notification settings - Fork 78
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
A collection of proposed enhancements #28
Open
slcasner
wants to merge
14
commits into
Helius:master
Choose a base branch
from
slcasner:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
The trailing whitespace gets removed by editing steps I have done for other changes, so I wanted to commit this change by itself first to avoid making the other changes less clear.
Instead of separate memset calls and assignments to initialize various members of the microrl_t struct to zero, just clear the entire struct first and then set the nonzero values.
1. Add a 'void* userdata' member to microrl_t that can be used by applications to store a C++ object pointer or other context info. 2. Pass the pointer to microrl_t in all callbacks so that the operations can be specific to a particular instance of microrl. 3. Enclose the definitions in microrl.h in a conditional 'extern "C"' block to allow those definitions to be referenced from C++ files.
This is a change that was missed in copying over to this repository in the previous commit 7334fbd.
Change the example code to include the microrl_t pointer as the first argument of all the callbacks as was added to the microrl library in commit 7334fbd. Note: The changes in avr_misc have not been tested since I don't have a build environment.
Typing multiple Ctrl-D while on an empty command line would crash. This was missed in merging changes from another repository.
When used over a network connection, each print() call in microrl may be transmitted as a separate packet, so the many small print operations make output slow. This commit optimizes some common cases to avoid unnecessary prints and combines others into a buffer to be printed together. - When adding characters to the end of a command, now just the single character is output. This avoids cursor jumping. Similarly, when backspacing at the end of the line, just the simple sequence back-one, space, back-one is sufficient. - The biggest number of back-to-back print operations occurred in terminal_print_line() where each character was printed separately. Now the characters are packed into a temporary buffer first so they can be printed in one or a few operations, depending upon the command length and buffer size. Positioning sequences are also consolidated into the buffer. - Some calls to terminal_reset_cursor() that were changed to call terminal_move_cursor() instead since the latter outputs fewer characters and avoids extra jumping of the cursor on slow lines. The code to reset the cursor position was then incorporated into terminal_print_line() where those positioning operations can be consolidated into a print buffer. Since the reset is not always necessary, an argument controls whether it is done. - A new define _USE_CARRIAGE_RETURN controls using a carriate return to reposition the cursor to the left margin rather than moving left by a large number. This takes fewer characters. This optimization can be turned off if the terminal simulates receiving a linefeed when it receives the carriage return. - In terminal_print_line(), the delete-to-end-of-line escape sequence is moved to be output after all of the command text. This avoids a flash of the text going away and then being repainted. - The code to create repositioning sequences was extracted into a separate function generate_move_cursor() so it can be shared between terminal_move_cursor() and terminal_print_line(). - After reorganizing the code to generate repositioning strings there was only one call to u16bit_to_str(), so it was merged inline. - strcpy() calls of just a few characters are replaced by depositing the characters individually. This removes a dependency and may use fewer instructions. - To allow ^U to backspace across all of the characters to the left of the curso in one motion, microrl_backspace() now takes a parameter for the number of spaces to move rather than always moving one.
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.
I have added several enhancements to microrl as we are using it in the Open Vehicle Monitoring System project. I have separated those enhancements into separate commits in my repository here and arranged them with the ones least likely to raise objections applied first.