Supress undef ERROR

[flagarde]

Hi,

This changes allow to remove the #undef ERROR and to move most of the platform hearders to the cpp

[staxfur]

Hey, thank you really much for this PR! This is one of the most important things on our roadmap currently. I'll test it in the next days to make sure that all things still work.

[staxfur]

I have done testing on linux now and there are problems when exiting the raw mode of the terminal, sometimes it doesn't work and sometimes it works, but the terminal is not reverted back into it's original state completely. I'll investigate this, the current master doesn't have these issues. Do you have tested all examples on windows? @flagarde

I'll look into fixing the linux side, when Windows does work well this should be simple.

[flagarde]

@MCWertGaming I have tested a bit on windows yes. It's compile and seems to have the same behaviour than before. On windows in didn't touch the code itselft so it should be fine.

On Linux trying menu and doing CTRL-C I loose the caret too if it's what you mean ?

[certik]

Thanks @flagarde, this is indeed needed. Once the bugs are fixed, this can go in. Thanks @MCWertGaming for testing this!

[staxfur]

Okay, thank you for testing @flagarde, I'll do the testing on windows then as well. Especially the multiline prompt makes problems sometimes, so we should make sure that everything works well. I'll push the fix ones it's ready. Then I'll test windows and if that works, we are ready to merge! Thank you again.

The PR is tracked in #159.

[flagarde]

@MCWertGaming on Windows the only changes have been done to move the headers to the cpp files so it should not change the behaviour.

On Linux and I guess on Windows I think we always have a problem if we use signal handling to quit the program.

[staxfur]

@flagarde Windows and Linux works well because both systems have well build in support for the "raw mode" of the terminal that deactivates the terminal build in handling of user input and that stuff so we can process them in our application ourselves. This works fine, but I think that your change of the termio struct of the terminal class could have broken the terminal restoring. I'll look into this now.

[staxfur]

I have investigated the issues on Linux and came to the conclusion that it's just the window class from cpp-terminal/window.cpp that is broken. On linux it basically doesn't revert the terminal state back to normal. When I debugged the code, I noticed that calling the render function from the window class changes the termio struct from:

c_iflag = {tcflag_t} 1280
c_oflag = {tcflag_t} 5
c_cflag = {tcflag_t} 191
c_lflag = {tcflag_t} 35387
c_line = {cc_t} 0 '\000'

to:

c_iflag = {tcflag_t} 1280
c_oflag = {tcflag_t} 5
c_cflag = {tcflag_t} 1431777296
c_lflag = {tcflag_t} 21845
c_line = {cc_t} 99 'c'

Where we can clearly see that the value change that will later be used for reverting the changes done to the terminal.
Also clang-tidy is showing the following notice on the orig_termios{*std::unique_ptr<termios>(new termios).get()} initialization of the termios struct:

Initializing pointer member 'orig_termios' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object

Is it possible that the orig_termios pointer points to a memory location that holds data, but isn't active anymore and then get's overwritten by the window class because it allocates a huge array?

I'm still not sure what the problem is. @flagarde could you look into this?

[staxfur]

I think that we shouldn't use pointers here, it'll cause a lot of trouble this way. What about either creating our own struct for termios, since it looks like this:

#define NCCS 32
struct termios
  {
    tcflag_t c_iflag;		/* input mode flags */
    tcflag_t c_oflag;		/* output mode flags */
    tcflag_t c_cflag;		/* control mode flags */
    tcflag_t c_lflag;		/* local mode flags */
    cc_t c_line;			/* line discipline */
    cc_t c_cc[NCCS];		/* control characters */
    speed_t c_ispeed;		/* input speed */
    speed_t c_ospeed;		/* output speed */
#define _HAVE_STRUCT_TERMIOS_C_ISPEED 1
#define _HAVE_STRUCT_TERMIOS_C_OSPEED 1
  };

So we would create a new struct with the same variables and then copy the data over from the termios struct for storing it and later copying it back into the original struct.

Or we could try to imclude the bits/termios-struct.h header. This will be a bit complicated, because it creates a compile error when _TERMIOS_H is not defined and the variable macros are not defined.

The types the struct uses are unsigned int and char basically.

[flagarde]

@MCWertGaming I agree we could create our own struct but I'm not sure all have the same type for tcflag_t etc... This is required by the standard ?

And as you mention, they don't want use to include only part of the termios stuffs

I moved from const termios & to a unique_ptr this solved the warning on clang and I had to use a trick to avoid some problem in the destructor.

[staxfur]

@flagarde The types used by termios are standard types that got renamed (like on windows, where they have DWORD, which is a unsigned in and BYTE which is a char etc). So it would be possible of course. I'll test you changes though first. Thank you for looking into this!

[staxfur]

The codeQL check is failing because version 1 of it is deprecated soon. Sorry for that. I'll fix that separately. Just that you know.

[staxfur]

Seems like the CodeQL check worked now? Anyway, my testing on Linux succeeded, I think that this is fine now. I'll confirm that everything works on windows as well. When that works without a problem as well, this can be merged. Thank you really much for the fix @flagarde.

[staxfur]

I have done the Windows testing now and didn't found any new bugs in this branch. I'll merge it then. Really nice to see this project moving forward!