From 155c877987e430ab8bae3ddc22aaa4a9256a7a72 Mon Sep 17 00:00:00 2001 From: Jon Michael Aanes Date: Fri, 14 Apr 2017 12:19:23 +0200 Subject: [PATCH] Alternative process for determining short tables This one is based on the representative width of the table. Not only does this produce better results, but it's also more futureproof. --- analyze_structure.lua | 10 ++- pretty.lua | 151 ++++++++++++--------------------------- test/test_function.lua | 4 +- test/test_pretty.lua | 20 +++--- test/test_resilience.lua | 20 ++++-- 5 files changed, 79 insertions(+), 126 deletions(-) diff --git a/analyze_structure.lua b/analyze_structure.lua index 89707a9..93bbcb4 100644 --- a/analyze_structure.lua +++ b/analyze_structure.lua @@ -71,7 +71,6 @@ local function get_value_types (t) return types end - local function largest_number_index (t) -- Returns the largest number index in t. @@ -84,6 +83,14 @@ local function largest_number_index (t) return max_index end +local function nr_elements_in_map (t) + local k, count = nil, -1 + repeat + k, count = next(t, k), count + 1 + until not k + return count +end + -------------------------------------------------------------------------------- local function contains_only_nice_string_keys (t) @@ -200,6 +207,7 @@ local function get_table_info (t) info.is_set = is_set(t) info.is_tabular = is_tabular(t) info.is_short = is_short_table(t) + info.nr_elems = nr_elements_in_map(t) -- TODO: Use this for something. -- Determine type of table if not info.has_seq and not info.has_map then info.type = TABLE_TYPE.EMPTY diff --git a/pretty.lua b/pretty.lua index ff72157..0238616 100644 --- a/pretty.lua +++ b/pretty.lua @@ -33,9 +33,10 @@ local SINGLE_LINE_TABLE_TYPES = { [TABLE_TYPE.STRING_MAP] = true, } -local SINGLE_LINE_SEQ_MAX_ELEMENTS = 10 -local SINGLE_LINE_MAP_MAX_ELEMENTS = 5 -local NR_CHARS_IN_LONG_STRING = 40 +local SINGLE_LINE_SEQ_MAX_ELEMENTS = 10 +local SINGLE_LINE_MAP_MAX_ELEMENTS = 5 +local NR_CHARS_IN_LONG_STRING = 40 +local MAX_WIDTH_FOR_SINGLE_LINE_TABLE = 38 local KEY_TYPE_SORT_ORDER = { ['number'] = 0, @@ -171,14 +172,6 @@ local function fill_holes_in_key_value_pairs (key_value_pairs) table.sort(key_value_pairs, compare_key_value_pairs) end -local function nr_elements_in_map (t) - local k, count = nil, -1 - repeat - k, count = next(t, k), count + 1 - until not k - return count -end - local function is_identifier(str) -- An identier is defined in the lua reference guide @@ -222,7 +215,7 @@ end local function width_of_strings_in_l (l, start_i, end_i) local width = 0 for i = start_i or 1, (end_i or #l) do - width = width + #l[i] + width = width + ((type(l[i]) ~= 'string') and 1 or #l[i]) end return width end @@ -236,9 +229,11 @@ local function ignore_alignment_info (l, start_i, stop_i) end local function fix_alignment (l, start_i, stop_i) + local start_i, stop_i = start_i or 1, stop_i or #l + -- Find maximums local max = {} - for i = start_i or 1, stop_i or #l do + for i = start_i, stop_i do if type(l[i]) == 'table' then max[ l[i][2] ] = math.max( l[i][1], max[ l[i][2] ] or 0 ) end @@ -252,16 +247,23 @@ local function fix_alignment (l, start_i, stop_i) end end +local function replace_seperator_info (l, replace_with, indent_char, depth, start_i, stop_i) + for i = start_i or 1, stop_i or #l do + if type(l[i]) ~= 'table' then + -- Do nothing + elseif l[i][1] == 'seperator' then + l[i] = replace_with .. indent_char:rep(depth) + elseif l[i][1] == 'indent' then + l[i], depth = '', depth + 1 + elseif l[i][1] == 'unindent' then + l[i], depth = '', depth - 1 + end + end +end + -------------------------------------------------------------------------------- -- Identifyer stuff -local SIMPLE_VALUE_TYPES = { - ['nil'] = true, - ['boolean'] = true, - ['number'] = true, - ['string'] = true, -} - local function is_empty_table (value) if type(value) ~= 'table' then error(('[pretty/internal]: Only tables allowed in function pretty.is_empty_table, but was given %s (%s)'):format(value, type(value)), 2) @@ -269,38 +271,6 @@ local function is_empty_table (value) return next(value) == nil end -local function is_short_table (value) - -- In this context, a short table is either an empty table, or one with a - -- single element. - - if type(value) ~= 'table' then - error(('[pretty/internal]: Only tables allowed in function pretty.is_short_table, but was given %s (%s)'):format(value, type(value)), 2) - end - - local first_key = next(value) - return (not first_key or SIMPLE_VALUE_TYPES[type(value[first_key])]) - and (next(value, first_key) == nil) -end - -local function is_simple_value (value) - -- In this context, a simple value is a either nil, a boolean, a number, - -- a string or a short table. - -- TODO: Add clause about long strings. (Maybe >7 chars?) - - --if type(value) == 'table' then print(value, is_short_table(value)) end - return SIMPLE_VALUE_TYPES[ type(value) ] - or type(value) == 'table' and is_short_table(value) -end - -local function contains_non_simple_key_or_value (t) - for k, v in pairs(t) do - if not is_simple_value(k) or not is_simple_value(v) then - return true - end - end - return false -end - local function get_table_type (value) -- Determines table type: -- * Sequence: All keys are integer in the range 1..#value @@ -321,21 +291,6 @@ local function get_table_type (value) end end -local function is_single_line_table (value) - -- In this context, a single-line table, is: - -- A) Either a sequence or a pure map. - -- B) Has no non-simple keys or values - -- C 1) If sequence, has at most SINGLE_LINE_SEQ_MAX_ELEMENTS elements. - -- C 2) If map, has at most SINGLE_LINE_MAP_MAX_ELEMENTS elements. - - local table_type = get_table_type(value) - - return not contains_non_simple_key_or_value(value) - and SINGLE_LINE_TABLE_TYPES[table_type] - and #value <= SINGLE_LINE_SEQ_MAX_ELEMENTS - and nr_elements_in_map(value) <= SINGLE_LINE_MAP_MAX_ELEMENTS -end - -------------------------------------------------------------------------------- -- Formatting stuff @@ -353,7 +308,7 @@ end local function format_key_and_value_arbitr_map (l, key, value, options, depth) local index_before_key = #l+1 l[#l+1] = '[' - format_value(key, options, 'max', l) + format_value(key, options, 'max', l) -- TODO: Outphase the usage of the "max" depth thingy. l[#l+1] = ']' l[#l+1] = { width_of_strings_in_l(l, index_before_key), 'key' } l[#l+1] = ' = ' @@ -375,7 +330,7 @@ local TABLE_TYPE_TO_PAIR_FORMAT = { -- Formatting tables -local function format_single_line_map (t, options, l) +local function format_map (t, options, depth, l) -- NOTE: Assumes that the input table was pre-checked with `is_single_line_table()` local table_type = l.info[t] and l.info[t].type or get_table_type(t) -- FIXME: This is a temp fix @@ -385,48 +340,33 @@ local function format_single_line_map (t, options, l) end local pair_format_func = TABLE_TYPE_TO_PAIR_FORMAT[table_type] - l[#l+1] = '{ ' - local top_before = #l + local start_of_table_i = #l + 1 + l[#l+1] = '{' + l[#l+1] = {'indent'} + l[#l+1] = {'seperator'} for _, pair in ipairs(key_value_pairs) do - pair_format_func(l, pair[1], pair[2], options, 'max') - l[#l+1] = ', ' - end - - -- Ignore the "width of key"-shit - ignore_alignment_info(l, top_before, #l) - - if l[#l] == ', ' then l[#l] = nil end - l[#l+1] = ' }' -end - -local function format_map (t, options, depth, l) - -- TODO: l. - - local table_type = l.info[t] and l.info[t].type or get_table_type(t) -- FIXME: This is a temp fix - local key_value_pairs = get_key_value_pairs_in_proper_order(t) - if table_type == TABLE_TYPE.SEQUENCE and l.info[t].has_holes then - fill_holes_in_key_value_pairs(key_value_pairs) - end - local pair_format_func = TABLE_TYPE_TO_PAIR_FORMAT[table_type] - - l[#l+1] = '{\n' - local top_before = #l - for _, pair in pairs(key_value_pairs) do - l[#l+1] = options.indent:rep(depth + 1) pair_format_func(l, pair[1], pair[2], options, depth + 1) - l[#l+1] = ',\n' + l[#l+1] = ',' + l[#l+1] = {'seperator'} end - -- Fix whitespace alignment - fix_alignment(l, top_before, #l) - - -- Fix and cleanup - if l[#l] == ',\n' then l[#l] = nil end - l[#l+1] = '\n' - l[#l+1] = options.indent:rep(depth) + if l[#l][1] == 'seperator' then l[#l-1], l[#l] = nil, nil end + l[#l+1] = {'unindent'} + l[#l+1] = {'seperator'} l[#l+1] = '}' + local table_width = width_of_strings_in_l(l, start_of_table_i) + + if table_width <= MAX_WIDTH_FOR_SINGLE_LINE_TABLE then + -- Is short table: Ignore the "width of key"-shit + replace_seperator_info(l, ' ', '', 0, start_of_table_i) + ignore_alignment_info(l, start_of_table_i) + else + -- Is long table: Fix whitespace alignment + replace_seperator_info(l, '\n', options.indent, depth, start_of_table_i) + fix_alignment(l, start_of_table_i) + end end function format_table (t, options, depth, l) @@ -444,9 +384,6 @@ function format_table (t, options, depth, l) l.visited[t] = true - -- Single line? - if is_single_line_table(t) then return format_single_line_map(t, options, l) end - if depth == 'max' then l[#l+1] = '{...}'; return end -- Normal table diff --git a/test/test_function.lua b/test/test_function.lua index 02cfbb4..f611ec8 100644 --- a/test/test_function.lua +++ b/test/test_function.lua @@ -84,7 +84,7 @@ do adv_getlocal = true, input = { a = function () SOME_RANDOM_UPVALUE = true end }, options = { more_function_info = true }, - expect = '{\n\ta = function () ... end\n}', + expect = '{ a = function () ... end }', } @@ -232,7 +232,7 @@ format_test { format_test { input = { math.cos, math.sin, math.abs }, options = { short_builtins = true }, - expect = '{\n\tmath.cos,\n\tmath.sin,\n\tmath.abs\n}', + expect = '{ math.cos, math.sin, math.abs }', } -------------------------------------------------------------------------------- diff --git a/test/test_pretty.lua b/test/test_pretty.lua index 79f908f..f513cc5 100644 --- a/test/test_pretty.lua +++ b/test/test_pretty.lua @@ -217,7 +217,7 @@ format_test { -- Can include very small tables format_test { input = { {1, 2, 3}, {4, 5, 6} }, - expect = '{\n\t{ 1, 2, 3 },\n\t{ 4, 5, 6 }\n}', + expect = '{ { 1, 2, 3 }, { 4, 5, 6 } }', } format_test { @@ -232,17 +232,17 @@ format_test { format_test { input = { { {} } }, - expect = '{\n\t{ {} }\n}', + expect = '{ { {} } }', } format_test { input = { [{ 1, 2 }] = { 2, 1 } }, - expect = '{\n\t[{ 1, 2 }] = { 2, 1 }\n}', + expect = '{ [{ 1, 2 }] = { 2, 1 } }', } format_test { input = { { {1, 2}, {3, 4} }, {5, 6} }, - expect = '{\n\t{\n\t\t{ 1, 2 },\n\t\t{ 3, 4 }\n\t},\n\t{ 5, 6 }\n}', + expect = '{ { { 1, 2 }, { 3, 4 } }, { 5, 6 } }', } format_test { @@ -254,24 +254,24 @@ format_test { format_test { input = { { {1, 2}, {3, 4} }, {5, 6} }, options = { max_depth = 1 }, - expect = '{\n\t{...},\n\t{...}\n}', + expect = '{ {...}, {...} }', } format_test { input = { { {1, 2}, {3, 4} }, {5, 6} }, options = { max_depth = 2 }, - expect = '{\n\t{\n\t\t{...},\n\t\t{...}\n\t},\n\t{ 5, 6 }\n}', + expect = '{ { {...}, {...} }, { 5, 6 } }', } format_test { input = { { {1, 2}, {3, 4} }, {5, 6} }, options = { max_depth = 3 }, - expect = '{\n\t{\n\t\t{ 1, 2 },\n\t\t{ 3, 4 }\n\t},\n\t{ 5, 6 }\n}', + expect = '{ { { 1, 2 }, { 3, 4 } }, { 5, 6 } }', } format_test { input = { [{ {1,2}, {3,4} }] = 'Hello World' }, - expect = '{\n\t[{...}] = \'Hello World\'\n}', + expect = '{ [{...}] = \'Hello World\' }', } format_test { @@ -281,13 +281,13 @@ format_test { format_test { input = { [true] = 1, [1] = false }, - expect = '{\n\t[1] = false,\n\t[true] = 1\n}', + expect = '{ [1] = false, [true] = 1 }', } format_test { -- Proper indent input = { [1] = 1, ['whatever'] = false }, - expect = '{\n\t[1] = 1,\n\t[\'whatever\'] = false\n}', + expect = '{ [1] = 1, [\'whatever\'] = false }', } format_test { diff --git a/test/test_resilience.lua b/test/test_resilience.lua index f6a8127..305ae22 100644 --- a/test/test_resilience.lua +++ b/test/test_resilience.lua @@ -46,9 +46,8 @@ end) SUITE:addTest('no_std_lib', function () -- This tests whether one could load the library with an empty env, without -- an error. - local chunk = loadfile('./library.lua') - setfenv(chunk, {}) - local library = chunk() + local env = {} + local library = setfenv(loadfile('./library.lua'), env)() for func, func_info in pairs(library) do error(('For some reason %s is defined in the library'):format(func_info.name)) @@ -58,13 +57,22 @@ end) SUITE:addTest('a_very_small_part_of_math', function () -- This tests whether one could load the library with an empty env, without -- an error. - local chunk = loadfile('./library.lua') - setfenv(chunk, { math = { abs = math.abs } }) - local library = chunk() + local env = { math = { abs = math.abs } } + local library = setfenv(loadfile('./library.lua'), env)() assert( library[math.abs], 'Why is math.abs not defined in the library?' ) end) +SUITE:addTest('redefined part of math', function () + -- This tests whether the library ignores a redefined function of an + -- builtin, when defining documentation. + local env = { setfenv = setfenv, loadfile = function() end } + local library = setfenv(loadfile('./library.lua'), env)() + + assert( library[env.setfenv], 'Why is setfenv not defined in the library?' ) + assert( not library[env.loadfile], 'Why is loadfile defined in the library?' ) +end) + -------------------------------------------------------------------------------- return SUITE