Cleanup line reader code for CLI

[aollier]

These changes make the LineReader class a concrete class whose implementation depends on whether or not USE_READLINE is defined. This greatly simplifies keepassxc-cli.cpp.
The readline library's search and internal and public variables now conform to the CMake standard (https://cmake.org/cmake/help/latest/manual/cmake-developer.7.html).

If the readline library is not found, the LineReader class then depends on the TextStream class. The cli library also depends on the TextStream class, but only for its implementation (Export.cpp).
These relationships are now expressed between the line_reader, text_stream, and cli libraries.
Also, by putting the code of LineReader in the line_reader library, we isolate the use of USE_READLINE into the single line_reader library. The cli library does not use anymore the USE_READLINE definition.

Finally, the new version of the search modules allows the use of CMake's modern library dependency syntax.

The FindReadline.cmake module was written with the help of Le Chat (https://chat.mistral.ai/chat).

Type of change

• ✅ Refactor (significant modification to existing code)

[varjolintu]

Is the NCurses dependency really needed for this? I don't really get the idea behind this whole change.

[aollier]

Hello @varjolintu and thank you for reviewing my pull request.

Your question touches on two points:

1. According to https://vcpkg.io/en/package/readline and https://vcpkg.io/en/package/readline-unix.html, the readline library depends on ncurses on Unix only. That's why I introduced the FindNcurses module. Traces of this dependency can be seen in the current code with the Ncurses_LIBRARY variable:
   

   keepassxc/cmake/FindReadline.cmake

   Lines 36 to 37 in 7c7ca45

   	if(Readline_INCLUDE_DIR AND Readline_LIBRARY AND Ncurses_LIBRARY)
   	set(READLINE_FOUND TRUE)

2. The idea behind this whole change is the simplification of src/cli/keepassxc-cli.cpp and the elimination of unnecessary classes SimpleLineReader and ReadlineLineReader. This also sets the USE_READLINE definition only where it is necessary: for the LineReader class only, which is now isolated. So the cli library does not carry the USE_READLINE definition anymore. By introducing two new libraries, these changes also clarify the dependency of the code: the keepassxc_core library does not require TextStream.cpp. cli library does with Export.cpp, and LineReader may depend on it if the readline library dependency is not found.

[aollier]

I've just realized that it is not necessary to introduce the FindNcurses module because it already exists one in the CMake modules library: https://cmake.org/cmake/help/latest/module/FindCurses.html.

On the other hand, I admit that I have not found any trace of the readline library's dependency on the ncurses library anywhere other than on vcpkg and the Ncurses_LIBRARY variable. What is your experience with this?

[aollier]

I finally found the repository of the readline library: git://git.git.savannah.gnu.org/readline.git.
In the INSTALL file, I can read this:

`--with-curses'
This tells readline that it can find the termcap library functions
(tgetent, et al.) in the curses library, rather than a separate
termcap library. Readline uses the termcap functions, but does not
usually link with the termcap or curses library itself, allowing
applications which link with readline the to choose an appropriate
library. This option tells readline to link the example programs with
the curses library rather than libtermcap.

This is an option passed to the configure script. So it seems that the (n)curses library may not be mandatory, if the --with-curses option is NOT passed to the configure script.

So what is your recommandation with this change? Should I let the FindReadline module unchanged even if on the vcpkg website we can see that the readline library requires the ncurses library?

[droidmonkey]

We do not use vcpkg on linux. If ncurses is a dependency of readline on macOS then finding the readline library would be sufficient as that should also bring in the ncurses library as needed. If we dont directly utilize a library than we shouldn't need to "find" it and link to it directly.

[aollier]

Ok I've updated my change by removing the FindNcurses module and every reference to the ncurses library.
I think it is correct now.

[droidmonkey]

@aollier sorry for the delay, will review this today

[aollier]

Thank you @droidmonkey for reviewing my pull request. 😃

[aollier]

@droidmonkey I've just found PR #8918 which seemed to be a good replacement for readline and that you found "outstanding" but this PR seems to have been forgotten. Would you like me to use linenoise instead of readline in this pull request?

[aollier]

The healthiest and most rigorous approach would be to switch libraries in a separate pull request. This one redefines the architecture. Switching dependencies will be very easy afterwards.