Skip to content

Conversation

@starseeker
Copy link
Contributor

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.

Implement a way to allow the previous API as seen in the old example.cpp to
keep working.
Fix from Chris McGregor (f4alt <christopherjmacgregor@gmail.com>:
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.
@starseeker
Copy link
Contributor Author

starseeker commented Mar 10, 2025

@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...

@yhirose
Copy link
Owner

yhirose commented Mar 10, 2025

@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. :)

@yhirose
Copy link
Owner

yhirose commented Mar 10, 2025

@starseeker I just quickly walked through the changes, and they basically look very good to me!

I left some comments regarding the followings:

  1. Missing inline keyword before some free function bodies and instance method bodies.
  2. Naming of private member variables of linenoiseState
  3. The history behavior

@starseeker
Copy link
Contributor Author

@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.

@yhirose
Copy link
Owner

yhirose commented Mar 11, 2025

@starseeker looks very good! Just one more change please. Could you rename main.cpp f1.cpp f2.cpp to multiinclude_main.cpp multiinclude_f1.cpp multiinclude_f2.cpp?

EDIT: multiinclude/main.cpp multiinclude/f1.cpp multiinclude/f2.cpp are actually better.

@starseeker
Copy link
Contributor Author

All set - moved to subdirectory.

@yhirose yhirose merged commit 5d2183e into yhirose:master Mar 11, 2025
@yhirose
Copy link
Owner

yhirose commented Mar 11, 2025

@starseeker thanks for your hard work!

@gyulamad
Copy link

wow. that was quick! thank you guys :)
@yhirose @starseeker

@starseeker starseeker deleted the class branch March 20, 2025 16:14
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.

3 participants