de-complicate the library

[staxfur]

At investigating a better solution to the sscanf problem I notices that the code is really complicated (or at least to me). I would be happy to "de-complicate" the code by changing workarounds around some things to shorter and more simple solutions (for example you created two while loops inside get_cursor_position() with creating the i var above and are using it inside both while loops and outside. I would like to make both while loops into for loops and move all parts which need the i car inside that loop. The contents of the same functions are really complicated too, like you created inline functions like write(), but it's contents was just std::cout << s << std::flush; I thought you would just write something elsewhere but not into stdout. So why not just doing that immediately where its needed? It would help others to understand your code faster). I have to add that I'm new to ANSI and this library is the first I'm contributing to and i respect if you don't like some things in upstream.

Changes i would like to make:

• simply-size some functions and loops
• move the ANSI code into definitions (macros) instead of inline functions (better readability and less code)
• split class definition and function definitions (move the sources of all functions outside the class definition)
• that would be nice because the system dependent stuff with all the #ifdef makes it really hard to tell what functions are provided
• move all static member functions of base_terminal class outside into the Term namespace (if it isn't already, my IDE is a bit confused with all the system dependent code)
• (remove the slow functions, as I'm not sure if anyone need them but it's optional)
• try to remove the binary bit wise operators as they are not recommended in C++ (and clang-tidy note, that it should be avoided)
• adding default labels to switch statement to throw a runtime_error, if an illegal or not caught value somehow manages to happen
• trying on making get_term_size() static, as only windows needs members, but maybe you could just make a workaround (that would enable the usage without needing the object of it's class)
• maybe some more things

I just wanted to ask, if these changes are welcomed inside this project. As I will use this library for some small games I write to practice programming, I'm willing to improve it with my own ideas.

Greetings!
Damon Leven

[certik]

Hi Damon, I think these changes are probably fine. Anything that simplifies the code is always welcome. The best is to send a separate PR for each change. Do not lump logically independent changes into the same PR. That way we can finish and merge a PR quicker. Ondrej

…

On Sat, Nov 28, 2020, at 8:32 AM, Damon Leven wrote: At investigating a better solution to the sscanf problem I notices that the code is really complicated (or at least to me). I would be happy to "de-complicate" the code by changing workarounds around some things to shorter and more simple solutions (for example you created two while loops inside get_cursor_position() with creating the i var above and are using it inside both while loops and outside. I would like to make both while loops into for loops and move all parts which need the i car inside that loop. The contents of the same functions are really complicated too, like you created inline functions like `write()`, but it's contents was just `std::cout << s << std::flush;` I thought you would just write something elsewhere but not into stdout. So why not just doing that immediately where its needed? It would help others to understand your code faster). I have to add that I'm new to ANSI and this library is the first I'm contributing to and i respect if you don't like some things in upstream. Changes i would like to make: * simply-size some functions and loops * move the ANSI code into definitions (macros) instead of inline functions (better readability and less code) * split class definition and function definitions (move the sources of all functions outside the class definition) * that would be nice because the system dependent stuff with all the `#ifdef` makes it really hard to tell what functions are provided * move all static member functions of `base_terminal` class outside into the `Term` namespace (if it isn't already, my IDE is a bit confused with all the system dependent code) * (remove the slow functions, as I'm not sure if anyone need them but it's optional) * try to remove the binary bit wise operators as they are not recommended in C++ (and clang-tidy note, that it should be avoided) * adding default labels to switch statement to throw a runtime_error, if an illegal or not caught value somehow manages to happen * trying on making `get_term_size()` static, as only windows needs members, but maybe you could just make a workaround (that would enable the usage without needing the object of it's class) * maybe some more things I just wanted to ask, if these changes are welcomed inside this project. As I will use this library for some small games I write to practice programming, I'm willing to improve it with my own ideas. Greetings! Damon Leven — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#64>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWEKZUUHTPFFLJRDH4DSSEJZNANCNFSM4UF3OXOA>.

[staxfur]

Hi Ondrej, that's great! I will make it that way. Thank you!

Damon

[staxfur]

to give a small update to this pull-request, I'll split #68 into smaller parts so we can discuss it seperately instead of all in one.