Hi Dzmitry, On Thu, 23 Feb 2023 at 11:10, Dzmitry Sankouski <dsankou...@gmail.com> wrote: > > vidconsole_get_font_size is only used in tests and in font > command. It's role in 'font size' command was to only fetch > current font name, to be used in select font function. > This is redundant, because it's easy to check for empty > string, and reuse current name right in select function. > > Test functions in public API use memory and clutter interface. > > Move vidconsole_get_font_size to new cmd/test.h file. > Wrap it's implementation with #ifdef only when tests enabled. > > Signed-off-by: Dzmitry Sankouski <dsankou...@gmail.com> > --- > Changes for v2: N/A > Changes for v3: N/A > Charges for v4: N/A > Charges for v5: N/A > Charges for v6: N/A > > cmd/font.c | 5 ++--- > drivers/video/console_truetype.c | 8 +++++++- > include/cmd/test.h | 19 +++++++++++++++++++ > include/video_console.h | 9 --------- > test/cmd/font.c | 1 + > 5 files changed, 29 insertions(+), 13 deletions(-) > create mode 100644 include/cmd/test.h
I'm not a fan of this for three reasons: - it changes the current select_font() API, in which NULL currently means to select the default font, not the current one (although that seems to be broken in the code!) - we may want to allow checking the size of a font - don't want #ifdefs for test in console_truetype.c (tests should ideally just work with the normal plumbing) Regards, Simon