move prints to console out of help_db get_commands()

[dhthwy]

let try_autocomplete() do the printing.

Also may be worthwhile to use using for the result type or return a struct instead. Like a Result struct.

[ab9rf]

this function currently uses an out parameter - i would argue that this is a use case for returning a std::expected<std::vector<std::string>>, std::string> instead of having an out parameter

oh, nevermind, std::expected is c++23, so let's notate this for review when we move to c++23 -- which won't be prior to 2029 unless we decide to change our Linux support policy - while MSVC already supports std::expected, gcc doesn't until gcc 14, and we don't get gcc 14 until Ubuntu LTS 24.04 goes EOL in April 2029

[ab9rf]

thinking a bit more about this, i think this is a candidate for using an exception. the condition is exceptional and should rarely occur, so using an exception for the exceptional case is pretty much what exceptions are for

[dhthwy]

DFHack]# o
o is not recognized. Possible completions: once-per-save on-new-fortress open-legends orders overlay
[DFHack]# o
Cannot acquire core lock in helpdb.get_commands
o is not a recognized command.
[DFHack]# o

that's dfhack posix console. There's contention when fps is low.

[dhthwy]

Throwing is one possible way, but they should be caught. It wouldn't be nice to crash just because the simulation thread has a big workload that's dropped fps to 10 or happened to hold the lock for too long for some reason or another. If it happens at 10 fps (and often), it's sure to happen at higher fps (just less likely).

[dhthwy]

I'm not big on the std::optional approach for returning an error message.

std::expected would be cleaner, but as you said, it's not c++20.

Exception makes sense to handle it as well. I just wouldn't let it crash over a lag condition.

[ab9rf]

how do we feel about using TartanLlama's tl::expected?

[dhthwy]

I looked at it.

If you can stomach the dependency, it seems to be well supported in the community, as I'm sure you know.