-
Notifications
You must be signed in to change notification settings - Fork 61
Make linenoiseState a class with methods, various other adjustments #34
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
Conversation
Implement a way to allow the previous API as seen in the old example.cpp to keep working.
Fix from Chris McGregor (f4alt <[email protected]>: Old linenoise behavior would send commands on newline. For subprocess use, like we do with nirt in MGED, this is very important as the alternative means collecting commands like "Command_0\nCommnand_1.." and expecting the process to parse the newlines.
|
@yhirose A (somewhat belated) thought - if you're not really looking to change cpp-linenoise further, would it be better to set up a fork project and use that to pull in changes and do new work? Don't want to shove stuff at you that wouldn't be a benefit... |
Don't worry about it. Any improvements are welcome. :) |
|
@starseeker I just quickly walked through the changes, and they basically look very good to me! I left some comments regarding the followings:
|
Also add a compilation test case to examples to verify we can include more than once and create an executable.
|
@yhirose I think that's got it? Let me know if there's anything else. I'll look again at the history changes and submit a separate pull request if I can confirm whether there was a functional implication there. |
|
@starseeker looks very good! Just one more change please. Could you rename EDIT: |
|
All set - moved to subdirectory. |
|
@starseeker thanks for your hard work! |
|
wow. that was quick! thank you guys :) |
Follow on pull request from #33 that attempts to minimize the formatting changes. Also attempts to add a way to keep the old API (or at least the key parts of it) working - both the old version of the example and the updated version have been compiled against it.
Note that, unlike that PR, this one does not incorporate the #32 fixes - those are still of interest, but since that PR is still open we'll skip pulling it in here.
Might still be a few more tweaks to make (method name cleanups, etc.) but submitting in its current form to get things moving.