search

Eigenbahn / ai-dungeon-cli

:european_castle: A cli client to play.aidungeon.io
MIT License
150 stars 34 forks source link

Autocompletion of commands added #13

Closed otreblan closed 4 years ago

otreblan commented 4 years ago

Peek 29-02-2020 19-18

p3r7 commented 4 years ago

Added types

Nice work here.

I would have preferred 2 branches for the 2 separate features but it's not too dramatic.

Enhanced prompt with autocompletion

That's quite a nice feature.

Some tests

Tested your branch with different term settings.

The good: it even works flawlessly with TERM=vt320 on actual hardware terminal.

ac_vt320

The bad: it totally messes the input with TERM=dumb. At every prompt each key stroke is multiplied by the number of previous prompt having occured before. Furthermore, backspace doesn't work.

So we'd need to have a fallback to readline for this latter use-case (like we do for the splash logo).

Minor code style recommendation

I'm not super fan of having the input() method as well as the inputSession / completer vars as properties of AiDungeon class. We should restrict this class to game state and game logic.

Maintaining separation of concerns is key if we want to keep our sanity while this project grows.

For now, all the user input / output stuff is above under the # UTILS: TERMINAL comment. We could start wrapping this up into a dedicated class instead of relying on global vars.

Please note that https://github.com/Eigenbahn/ai-dungeon-cli/pull/9 suggests using the package as a lib and using setters to override these user interaction functions. This would allow building stuff such as chat bots.