From d22e138413bfefc692b5f43dafd2153a07f5286b Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Sun, 11 Sep 2022 14:57:31 +0100 Subject: [PATCH] Fix numerous off-by-one errors in help program We clamped various values to the height of the screen, rather than the height of the content box (height-1). We didn't notice this most of the time as the last line of a file is empty - it only really mattered when a file was the same height as the computer's screen. We now do the following: - Strip the trailing new line from a file when reading. - Replace most usages of height with height-1. --- .../computercraft/lua/rom/programs/help.lua | 22 ++++++----- .../test-rom/spec/programs/help_spec.lua | 38 +++++++++++++++++++ .../resources/test-rom/spec/test_helpers.lua | 15 ++++++++ 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/src/main/resources/data/computercraft/lua/rom/programs/help.lua b/src/main/resources/data/computercraft/lua/rom/programs/help.lua index 22c70f83e..14fd9d261 100644 --- a/src/main/resources/data/computercraft/lua/rom/programs/help.lua +++ b/src/main/resources/data/computercraft/lua/rom/programs/help.lua @@ -146,14 +146,17 @@ end local contents = file:read("*a") file:close() +-- Trim trailing newlines from the file to avoid displaying a blank line. +if contents:sub(-1) == "\n" then contents:sub(1, -2) end local word_wrap = sFile:sub(-3) == ".md" and word_wrap_markdown or word_wrap_basic local width, height = term.getSize() +local content_height = height - 1 -- Height of the content box. local lines, fg, bg, sections = word_wrap(contents, width) local print_height = #lines -- If we fit within the screen, just display without pagination. -if print_height <= height then +if print_height <= content_height then local _, y = term.getCursorPos() for i = 1, print_height do if y + i - 1 > height then @@ -201,7 +204,7 @@ end local function draw() - for y = 1, height - 1 do + for y = 1, content_height do term.setCursorPos(1, y) if y + offset > print_height then -- Should only happen if we resize the terminal to a larger one @@ -228,14 +231,14 @@ while true do if param == keys.up and offset > 0 then offset = offset - 1 draw() - elseif param == keys.down and offset < print_height - height then + elseif param == keys.down and offset < print_height - content_height then offset = offset + 1 draw() elseif param == keys.pageUp and offset > 0 then - offset = math.max(offset - height + 2, 0) + offset = math.max(offset - content_height + 1, 0) draw() - elseif param == keys.pageDown and offset < print_height - height then - offset = math.min(offset + height - 2, print_height - height) + elseif param == keys.pageDown and offset < print_height - content_height then + offset = math.min(offset + content_height - 1, print_height - content_height) draw() elseif param == keys.home then offset = 0 @@ -247,7 +250,7 @@ while true do offset = sections[current_section + 1].offset draw() elseif param == keys["end"] then - offset = print_height - height + offset = print_height - content_height draw() elseif param == keys.q then sleep(0) -- Super janky, but consumes stray "char" events. @@ -257,7 +260,7 @@ while true do if param < 0 and offset > 0 then offset = offset - 1 draw() - elseif param > 0 and offset < print_height - height then + elseif param > 0 and offset <= print_height - content_height then offset = offset + 1 draw() end @@ -270,7 +273,8 @@ while true do end width, height = new_width, new_height - offset = math.max(math.min(offset, print_height - height), 0) + content_height = height - 1 + offset = math.max(math.min(offset, print_height - content_height), 0) draw() draw_menu() elseif event == "terminate" then diff --git a/src/test/resources/test-rom/spec/programs/help_spec.lua b/src/test/resources/test-rom/spec/programs/help_spec.lua index 35ce021b4..9ce15a102 100644 --- a/src/test/resources/test-rom/spec/programs/help_spec.lua +++ b/src/test/resources/test-rom/spec/programs/help_spec.lua @@ -1,8 +1,46 @@ local capture = require "test_helpers".capture_program +local with_window_lines = require "test_helpers".with_window_lines describe("The help program", function() + local function stub_help(content) + local name = "/help_file.txt" + io.open(name, "wb"):write(content):close() + stub(help, "lookup", function() return name end) + end + + local function capture_help(width, height, content) + stub_help(content) + + local co = coroutine.create(shell.run) + local window = with_window_lines(width, height, function() + local ok, err = coroutine.resume(co, "help topic") + if not ok then error(err, 0) end + end) + return coroutine.status(co) == "dead", window + end + it("errors when there is no such help file", function() expect(capture(stub, "help nothing")) :matches { ok = true, error = "No help available\n", output = "" } end) + + it("prints a short file directly", function() + local dead, output = capture_help(10, 3, "a short\nfile") + expect(dead):eq(true) + expect(output):same { + "a short ", + "file ", + " ", + } + end) + + it("launches the viewer for a longer file", function() + local dead, output = capture_help(10, 3, "a longer\nfile\nwith content") + expect(dead):eq(false) + expect(output):same { + "a longer ", + "file ", + "Help: topi", + } + end) end) diff --git a/src/test/resources/test-rom/spec/test_helpers.lua b/src/test/resources/test-rom/spec/test_helpers.lua index 5d1ba9f1c..d11a33b3d 100644 --- a/src/test/resources/test-rom/spec/test_helpers.lua +++ b/src/test/resources/test-rom/spec/test_helpers.lua @@ -56,7 +56,22 @@ local function with_window(width, height, fn) return redirect end +--- Run a function redirecting to a new window with the given dimensions, +-- returning the content of the window. +-- +-- @tparam number width The window's width +-- @tparam number height The window's height +-- @tparam function() fn The action to run +-- @treturn {string...} The content of the window. +local function with_window_lines(width, height, fn) + local window = with_window(width, height, fn) + local out = {} + for i = 1, height do out[i] = window.getLine(i) end + return out +end + return { capture_program = capture_program, with_window = with_window, + with_window_lines = with_window_lines, }