Skip to content

Make state a C++ class, formatting, other minor changes#33

Closed
starseeker wants to merge 16 commits intoyhirose:masterfrom
starseeker:rework
Closed

Make state a C++ class, formatting, other minor changes#33
starseeker wants to merge 16 commits intoyhirose:masterfrom
starseeker:rework

Conversation

@starseeker
Copy link
Copy Markdown
Contributor

Ran things through clang-format, and reworked the linenoise functions operating on the struct to be methods on a class. Doubt it's of too much interest since it diverges things a fair bit from upstream linenoise, but figured I'd offer it just in case.

The line wiping addition is in support of trying to use this with a program that listens for input from another thread and needs to do a little orchestration to try and get the output displayed cleanly.

starseeker and others added 16 commits October 29, 2024 16:18
In this context, line wiping refers to temporarily removing the contents
of the current line from the screen, but not otherwise changing the line
editing state.  Its use case is temporarily removing the prompt to allow
other output to be written, with an intent to immediately restore it
once that output is complete.
- support unicode for windows
- fixed line wrapping
- Improved drawing by removing curso
This is an attempt at merging the changes from
yhirose#32
@gyulamad
Copy link
Copy Markdown

gyulamad commented Mar 7, 2025

Can't wait to get this in, I am facing with the exact same issue and started to modify the linenoise code in a similar manner but I wouldn't reinvent the wheel if this one has a chance to get merged. Any progress on it? @yhirose

@yhirose
Copy link
Copy Markdown
Owner

yhirose commented Mar 7, 2025

@starseeker could you exclude the Ran though clang-format commit from this pull request, so that I can easily review the changes? You may want to send another pull request later of this formatting change. Thanks!

@starseeker
Copy link
Copy Markdown
Contributor Author

starseeker commented Mar 7, 2025

@yhirose @gyulamad Glad to - do you have a particular format/formatter you prefer? My editor doesn't always do things quite the same way when it comes to autoformatting, so if you have something you want me to match I can try to be more careful.

I actually ended up taking things quite a ways further than this patch in the end - see https://github.com/BRL-CAD/brlcad/blob/main/src/libtermio/linenoise.hpp for where I ultimately landed. (It's been a little bit, but my broad recollection is turning everything that wasn't implementation detail into methods on a linenoiseState C++ class.)

I was focused on being able to do some specific things to get a relatively simple async I/O with multithreading usage scenario working (https://github.com/BRL-CAD/brlcad/blob/main/src/gtools/gsh/gsh.cpp), and I had figured my changes would be too extensive to be of interest to cpp-linenoise per say, but if there is interest what I'll try doing is "replaying" the key commits in a new branch and preparing a new pull request - would that work?

@starseeker
Copy link
Copy Markdown
Contributor Author

Update - working on replaying the changes at https://github.com/starseeker/cpp-linenoise/tree/class - once I've got a new PR I'll close this one and refer it to the new one.

@yhirose
Copy link
Copy Markdown
Owner

yhirose commented Mar 7, 2025

Glad to - do you have a particular format/formatter you prefer?

The default formatting style of clang-format is fine with me. The reason I asked to remove the last formatting commit is that it makes it hard for me to distinguish whether a diff is an actual change for the feature or a formatting change. I would like to focus on just the actual changes for the review.

Regarding the feature change, you are welcome to add and improve features in this repository, such as adding the state interface. However, I would like to request that the original interface be retained as well, so users don't need to change their code when updating cpp-linenoise. Is it possible to keep the original interface with a 'global' or 'default' state underneath?

had figured my changes would be too extensive to be of interest to cpp-linenoise per say, but if there is interest what I'll try doing is "replaying" the key commits in a new branch and preparing a new pull request - would that work?

Sounds good to me!

@starseeker
Copy link
Copy Markdown
Contributor Author

However, I would like to request that the original interface be retained as well, so users don't need to change their code when updating cpp-linenoise. Is it possible to keep the original interface with a 'global' or 'default' state underneath?

It should be, yes. I'll do some testing.

@starseeker
Copy link
Copy Markdown
Contributor Author

Closing in favor of #34

@starseeker starseeker closed this Mar 10, 2025
@starseeker starseeker deleted the rework 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