Coding style

General

• Indentation - 4 spaces
• Line Length - 100 characters as a rule of thumb, but please don't make code uglier and harder to grep with forced linebreaks to stick to an artificial limit
• If you use an autoformatter, try to limit it to only modifications. Most editors have a setting to that effect.

Naming

• Classnames - UpperCamelCase
• Functions - lowerCamelCase
• Variables - all_lower_case_with_underscores
• Constants - ALL_UPPER_CASE_WITH_UNDERSCORES
• Filenames - shortnouns

exceptions:

• global variables should be prefixed with G_: G_width
• boolean variables should be prefixed as predicates: is_directory
• i, k, v, and t are often used as follows:

for k,v in pairs(t) ... end
for i,v in ipairs(t) ... end
mt.__newindex = function(t, k, v) ... end

Strings in UI

Text strings in the program should normally use Sentence case, not Title Case. For further UI notes, see the documentation for InputDialog and related widgets.

Unit Tests

UI irrelevant features should be covered by test cases. We are using busted (https://olivinelabs.com/busted/) Lua test framework. You may find samples at https://github.com/koreader/koreader/tree/master/spec/unit (front) or https://github.com/koreader/koreader-base/tree/master/spec/unit (base).

We are lacking test cases heavily. So if you would like to help with source code, but cannot find an interesting feature to work on, adding test cases for existing components is really appreciated.

File insulation in busted is disabled by default. So to make other tests happy, please ensure to clean up any changes. E.g: modified modules, settings, temporary files, etc.

A simple way to ensure nothing left is to run busted manually with

./kodev make
cd koreader-emulator-x86_64-linux-gnu/koreader/
busted --lua='./luajit' \
       --no-auto-insulate \
       --lazy \
       --exclude-tags=notest \
       --shuffle \
       --repeat=10 \
       -o './spec/front/unit/verbose_print' \
       spec/front/unit

Replace 'front' with 'base' to execute tests in base.

Shuffle and repeat have not been enabled by default, because lots of tests violate the rule. But the failure tests should be fixed and these options will be enabled soon or later.

package.unload() and package.replace() in commonrequire may help to handle Lua packages.

Assertions

Lua provides both error() and assert() functions. (https://www.lua.org/pil/8.3.html) Though we do not use it too often, they are still very useful tools to help track unexpected situations and also to save other developers time of debugging.

• Assertions should be avoided in following situations.

-- Checking the input parameters of a function.

Usually this should be covered by the return value of a function.

-- System IO

Do not expect a file can always be opened.

-- loadfile / loadstring

An external source file may be edited unexpectedly, and causes trouble.

• Assertions are suggested in the following situations.

-- Unreasonable values in the middle of some logic. E.g.

local x = 100
local y = min(1, x)
assert(y <= 1, "Any reason y could be larger than 1?")

-- Delay referred variables. E.g.

function C:setConfig(config)
    self.config = config
end

function C:loadConfig()
    assert(self.config ~= nil, "Have you forgotten to call setConfig?")
end

or

local D = C:new{}

function D:makeChoice(p)
    if p then
        return D:doSomething()
    else
        return D:doSomethingElse()
    end
end

function D:doSomething() end

function D:doSomethingElse()
    assert(self.default_value, "Once makeChoice(false) is called, default_value is expected")
end

If the failure can be silently ignored, e.g. an optional component may malfunction, but user can still use major functions, a Dbg:dassert() is preferred. It's a no-op on end-users' devices.

Order of Requires

Please order the requires according to the following rules:

local Class = require("class")  -- Uppercase class
local Device = require("device")  -- Uppercase class
local func = require("func")  -- Lowercase function
local glob = require("glob")  -- Lowercase function
local _ = require("gettext")  -- Symbol
local T = require("ffi/util").template  -- One of the functions in a file
local V = require("utils2").version  -- The other function, ordered by ASCII.

The benefit of ordering requires is obvious: developers can easily find whether a dependency has been required already even when not using an advanced editor or on the web page, and reduces the possibility to redundantly require a dependency. Keeping the requires ordered is also not a heavy duty except for the first time.

Function naming

Names of functions should be a verb or a verb-noun phrase in first person tense, except for event handlers, which should be named as on + EventType.

-- If the function should be private, prefix it with an underscore.

-- If the function indicates a state and should be in passive tense or an adjective, the "is" or "are" can be omitted.

E.g.

function AWidget:onCloseWidget()
  -- An event handler to listen to "CloseWidget" event.
  self.saveSettings()
end

function AWidget:saveSettings()
  -- A regular function to save settings.
  if self:_settingsChanged() then
    self:_saveSettings()
  end
end

function AWidget:_settingsChanged()
  -- A private function to check whether the settings are changed.
  -- Using _areSettingsChanged() is also acceptable.
  if self:_settingsEmpty() then return false end
end

function AWidget:_settingsEmpty()
  -- A private function to check whether the settings are empty.
  -- Using _areSettingsEmpty() is also acceptable.
end

function AWidget:_saveSettings()
  -- A private function to do the real saving work.
end

More background

Lua does not provide event mechanism, so KOReader has its own event system. In short, a Widget can listen to the events by defining an "on + EventType" function. The implementation is in EventListener, WidgetContainer and UIManager.

Tips

Local variables

Use locals rather than globals whenever possible!

End terminator

Because "end" is a terminator for many different constructs, it can help the reader (especially in a large block) if a comment is used to clarify which construct is being terminated:

for i,v in ipairs(t) do
    if type(v) == "string" then
        ...lots of code here...
    end -- if string
end -- for each t

Check empty table

Determine if a table t is empty (including non-integer keys, which #t ignores):

    if next(t) == nil then ... end

Reference: http://lua-users.org/wiki/LuaStyleGuide