Fix the failed test case and some issues regarding variable naming.

There will still be one more commit to replace some of the screenshot
module code with available functionality from gears (e.g. the filesystem
module).
This commit is contained in:
Brian Sobulefsky 2022-02-03 21:20:50 -08:00 committed by Emmanuel Lepage Vallee
parent 5a7faa0010
commit 1fdefad750
2 changed files with 132 additions and 121 deletions

View File

@ -26,9 +26,9 @@ local screenshot = {}
local screenshot_methods = {} local screenshot_methods = {}
-- Configuration data -- Configuration data
local ss_dir = nil local module_default_directory = nil
local ss_prfx = nil local module_default_prefix = nil
local frame_color = gears.color("#000000") local module_default_frame_color = gears.color("#000000")
local initialized = nil local initialized = nil
@ -37,13 +37,13 @@ local initialized = nil
-- Can be expanded to read X environment variables or configuration files. -- Can be expanded to read X environment variables or configuration files.
local function get_default_dir() local function get_default_dir()
-- This can be expanded -- This can be expanded
local d = os.getenv("HOME") local home_dir = os.getenv("HOME")
if d then if home_dir then
d = string.gsub(d, '/*$', '/') .. 'Images/' home_dir = string.gsub(home_dir, '/*$', '/') .. 'Images/'
if os.execute("bash -c \"if [ -d \\\"" .. d .. "\\\" -a -w \\\"" .. d .. if os.execute("bash -c \"if [ -d \\\"" .. home_dir .. "\\\" -a -w \\\"" ..
"\\\" ] ; then exit 0 ; else exit 1 ; fi ;\"") then home_dir .. "\\\" ] ; then exit 0 ; else exit 1 ; fi ;\"") then
return d return home_dir
end end
end end
@ -79,10 +79,11 @@ local function check_directory(directory)
-- If we use single quotes, we only need to deal with single quotes - (I -- If we use single quotes, we only need to deal with single quotes - (I
-- promise that's meaningful if you think about it from a bash perspective) -- promise that's meaningful if you think about it from a bash perspective)
local dir = string.gsub(directory, "'", "'\\'\\\\\\'\\''") local bash_esc_dir = string.gsub(directory, "'", "'\\'\\\\\\'\\''")
if os.execute("bash -c 'if [ -d '\\''" .. dir .. "'\\'' -a -w '\\''" .. if os.execute("bash -c 'if [ -d '\\''" .. bash_esc_dir ..
dir .. "'\\'' ] ; then exit 0 ; else exit 1 ; fi'") then "'\\'' -a -w '\\''" .. bash_esc_dir ..
"'\\'' ] ; then exit 0 ; else exit 1 ; fi'") then
return directory return directory
else else
-- Currently returns nil if the requested directory string cannot be used. -- Currently returns nil if the requested directory string cannot be used.
@ -119,11 +120,11 @@ local function check_filepath(filepath)
-- options, this function is basically unchanged, except for trying -- options, this function is basically unchanged, except for trying
-- to match a regex for each supported format (e.g. (png|jpe?g|gif|bmp)) -- to match a regex for each supported format (e.g. (png|jpe?g|gif|bmp))
-- NOTE: we should maybe make this case insensitive too? -- NOTE: we should maybe make this case insensitive too?
local fname_start, fname_end = string.find(filepath,'/[^/]+%.png$') local filename_start, filename_end = string.find(filepath,'/[^/]+%.png$')
if fname_start and fname_end then if filename_start and filename_end then
local directory = string.sub(filepath, 1, fname_start) local directory = string.sub(filepath, 1, filename_start)
local file_name = string.sub(filepath, fname_start + 1, fname_end) local file_name = string.sub(filepath, filename_start + 1, filename_end)
directory = check_directory(directory) directory = check_directory(directory)
if directory then if directory then
return directory .. file_name return directory .. file_name
@ -141,18 +142,19 @@ end
local function parse_filepath(filepath) local function parse_filepath(filepath)
-- Same remark as above about adding more image formats in the future -- Same remark as above about adding more image formats in the future
local fname_start, fname_end = string.find(filepath,'/[^/]+%.png$') local filename_start, filename_end = string.find(filepath,'/[^/]+%.png$')
if fname_start and fname_end then if filename_start and filename_end then
local directory = string.sub(filepath, 1, fname_start) if filename_end - filename_start > 14 + 4 then
local file_name = string.sub(filepath, fname_start + 1, fname_end)
if fname_end - fname_start > 14 + 4 then local directory = string.sub(filepath, 1, filename_start)
local file_name = string.sub(filepath, filename_start + 1, filename_end)
local base_name = string.sub(file_name, 1, #file_name - 4) local base_name = string.sub(file_name, 1, #file_name - 4)
-- Is there a repeat count in Lua patterns, like {14} for most regexes? -- Is there a repeat count in Lua, like %d{14}, as for most regexes?
local date_start, date_end = string.find(base_name, '%d%d%d%d%d%d%d%d%d%d%d%d%d%d$') local date_start, date_end =
string.find(base_name, '%d%d%d%d%d%d%d%d%d%d%d%d%d%d$')
if date_start and date_end then if date_start and date_end then
return directory, string.sub(base_name, 1, date_start - 1) return directory, string.sub(base_name, 1, date_start - 1)
@ -178,45 +180,45 @@ end
-- unitialized state and without passing any arguments. -- unitialized state and without passing any arguments.
local function configure_path(directory, prefix) local function configure_path(directory, prefix)
local d, p local _directory, _prefix
if not initialized or directory then if not initialized or directory then
d = check_directory(directory) _directory = check_directory(directory)
if not d then if not _directory then
return return
end end
else else
d = ss_dir _directory = module_default_directory
end end
if not initialized or prefix then if not initialized or prefix then
p = check_prefix(prefix) _prefix = check_prefix(prefix)
if not p then if not _prefix then
return return
end end
else else
p = ss_prfx _prefix = module_default_prefix
end end
-- In the case of taking a screenshot in an unitialized state, store the -- In the case of taking a screenshot in an unitialized state, store the
-- configuration parameters and toggle to initialized going forward. -- configuration parameters and toggle to initialized going forward.
if not initialized then if not initialized then
ss_dir = d module_default_directory = _directory
ss_prfx = p module_default_prefix = _prefix
initilialized = true initilialized = true
end end
return d, p return _directory, _prefix
end end
local function make_filepath(directory, prefix) local function make_filepath(directory, prefix)
local dir, prfx local _directory, _prefix
local date_time = tostring(os.date("%Y%m%d%H%M%S")) local date_time = tostring(os.date("%Y%m%d%H%M%S"))
dir, prfx = configure_path(directory, prefix) _directory, _prefix = configure_path(directory, prefix)
local filepath = dir .. prfx .. date_time .. ".png" local filepath = _directory .. _prefix .. date_time .. ".png"
return dir, prfx, filepath return _directory, _prefix, filepath
end end
-- Internal function to do the actual work of taking a cropped screenshot -- Internal function to do the actual work of taking a cropped screenshot
@ -262,7 +264,7 @@ function show_frame(ss, geo)
ss._private.frame = ss._private.frame or wibox { ss._private.frame = ss._private.frame or wibox {
ontop = true, ontop = true,
bg = frame_color bg = ss._private.frame_color
} }
local frame = ss._private.frame local frame = ss._private.frame
@ -404,8 +406,6 @@ function screen_screenshot(ss)
if ss.screen then if ss.screen then
if type(ss.screen) == "number" then if type(ss.screen) == "number" then
ss._private.screen = capi.screen[ss.screen] or aw_screen.focused() ss._private.screen = capi.screen[ss.screen] or aw_screen.focused()
elseif ss.screen.index and type(ss.screen.index) == "number" then
ss._private.screen = capi.screen[ss.screen.index] or aw_screen.focused()
end end
else else
ss._private.screen = aw_screen.focused() ss._private.screen = aw_screen.focused()
@ -421,12 +421,12 @@ end
-- Internal function executed when a client window screenshot is taken. -- Internal function executed when a client window screenshot is taken.
function client_screenshot(ss) function client_screenshot(ss)
--
-- note the use of _private becuase client has no setter -- note the use of _private becuase client has no setter
if ss.client and ss.client.content then if not ss.client then
-- If the passed client arg has a content property we'll take it.
else
ss._private.client = capi.client.focus ss._private.client = capi.client.focus
end end
ss._private.geometry = ss.client:geometry() ss._private.geometry = ss.client:geometry()
ss:filepath_builder() ss:filepath_builder()
ss._private.surface = gears.surface(ss.client.content) -- surface has no setter ss._private.surface = gears.surface(ss.client.content) -- surface has no setter
@ -504,12 +504,13 @@ local default_method_name = "root"
-- @tparam[opt] table args Table passed with the configuration data. -- @tparam[opt] table args Table passed with the configuration data.
-- @treturn boolean true or false depending on success -- @treturn boolean true or false depending on success
function module.set_defaults(args) function module.set_defaults(args)
local tmp local tmp
tmp = check_directory(args.directory) tmp = check_directory(args.directory)
if tmp then if tmp then
ss_dir = tmp module_default_directory = tmp
else else
initialized = nil initialized = nil
return false return false
@ -518,7 +519,7 @@ function module.set_defaults(args)
tmp = check_prefix(args.prefix) tmp = check_prefix(args.prefix)
if tmp then if tmp then
ss_prfx = tmp module_default_prefix = tmp
else else
-- Should be unreachable as the default will always be taken -- Should be unreachable as the default will always be taken
initialized = nil initialized = nil
@ -536,6 +537,7 @@ function module.set_defaults(args)
end end
return true return true
end end
--- Take root window screenshots. --- Take root window screenshots.
@ -557,7 +559,7 @@ end
-- This is a wrapper constructor for a physical screen screenshot. See the main -- This is a wrapper constructor for a physical screen screenshot. See the main
-- constructor, new(), for details about the arguments. -- constructor, new(), for details about the arguments.
-- --
-- @staticfct awful.screenshot.screen -- @constructorfct awful.screenshot.screen
-- @tparam[opt] table args Table of arguments to pass to the constructor -- @tparam[opt] table args Table of arguments to pass to the constructor
-- @treturn screenshot The screenshot object -- @treturn screenshot The screenshot object
function module.screen(args) function module.screen(args)
@ -571,7 +573,7 @@ end
-- This is a wrapper constructor for a client window screenshot. See the main -- This is a wrapper constructor for a client window screenshot. See the main
-- constructor, new(), for details about the arguments. -- constructor, new(), for details about the arguments.
-- --
-- @staticfct awful.screenshot.client -- @constructorfct awful.screenshot.client
-- @tparam[opt] table args Table of arguments to pass to the constructor -- @tparam[opt] table args Table of arguments to pass to the constructor
-- @treturn screenshot The screenshot object -- @treturn screenshot The screenshot object
function module.client(args) function module.client(args)
@ -588,7 +590,7 @@ end
-- This is a wrapper constructor for a snipper tool screenshot. See the main -- This is a wrapper constructor for a snipper tool screenshot. See the main
-- constructor, new(), for details about the arguments. -- constructor, new(), for details about the arguments.
-- --
-- @staticfct awful.screenshot.snipper -- @constructorfct awful.screenshot.snipper
-- @tparam[opt] table args Table of arguments to pass to the constructor -- @tparam[opt] table args Table of arguments to pass to the constructor
-- @treturn screenshot The screenshot object -- @treturn screenshot The screenshot object
function module.snipper(args) function module.snipper(args)
@ -602,7 +604,7 @@ end
-- This is a wrapper constructor for a snip screenshot (defined geometry). See -- This is a wrapper constructor for a snip screenshot (defined geometry). See
-- the main constructor, new(), for details about the arguments. -- the main constructor, new(), for details about the arguments.
-- --
-- @staticfct awful.screenshot.snip -- @constructorfct awful.screenshot.snip
-- @tparam[opt] table args Table of arguments to pass to the constructor -- @tparam[opt] table args Table of arguments to pass to the constructor
-- @treturn screenshot The screenshot object -- @treturn screenshot The screenshot object
function module.snip(args) function module.snip(args)
@ -627,9 +629,9 @@ end
-- @tparam string directory The path to the screenshot directory -- @tparam string directory The path to the screenshot directory
function screenshot:set_directory(directory) function screenshot:set_directory(directory)
if type(directory) == "string" then if type(directory) == "string" then
local dir = check_directory(directory) local checked_dir = check_directory(directory)
if dir then if checked_dir then
self._private.directory = dir self._private.directory = checked_dir
end end
end end
end end
@ -647,9 +649,9 @@ end
-- @tparam string prefix The prefix prepended to screenshot files names. -- @tparam string prefix The prefix prepended to screenshot files names.
function screenshot:set_prefix(prefix) function screenshot:set_prefix(prefix)
if type(prefix) == "string" then if type(prefix) == "string" then
local prfx = check_prefix(prefix) local checked_prefix = check_prefix(prefix)
if prfx then if checked_prefix then
self._private.prefix = prfx self._private.prefix = checked_prefix
end end
end end
end end
@ -758,17 +760,13 @@ function screenshot:filepath_builder(args)
else else
if directory and type(directory) == "string" then if self.directory then
directory = check_directory(directory)
elseif self.directory then
directory = self._private.directory -- The setter ran check_directory() directory = self._private.directory -- The setter ran check_directory()
else else
directory = get_default_dir() directory = get_default_dir()
end end
if prefix and type(prefix) == "string" then if self.prefix then
prefix = check_prefix(prefix)
elseif self.prefix then
prefix = self._private.prefix -- The setter ran check_prefix() prefix = self._private.prefix -- The setter ran check_prefix()
else else
prefix = get_default_prefix() prefix = get_default_prefix()
@ -801,17 +799,17 @@ function screenshot:save(args)
end end
--- Screenshot constructor - it is possible to call this directly, but it is --- Screenshot constructor - it is possible to call this directly, but it is
-- recommended to use the helper constructors, such as awful.screenshot.root -- recommended to use the helper constructors, such as awful.screenshot.root
-- --
-- Possible args include: -- Possible args include:
-- directory [string]: the path to the screenshot directory -- directory [string]: the path to the screenshot directory
-- prefix [string]: the prefix to prepend to screenshot file names -- prefix [string]: the prefix to prepend to screenshot file names
-- frame_color: the color of the frame for a snipper tool -- frame_color [gears color]: the color of the frame for a snipper tool
-- method: the method of screenshot to take (i.e. root window, etc). -- method [string]: the method of screenshot to take (i.e. root window, etc).
-- screen: the screen for a physical screen screenshot -- screen [index or screen]: the screen for a physical screen screenshot
-- on_success_cb: the callback to run on the screenshot taken with -- on_success_cb [function]: the callback to run on the screenshot taken with
-- a snipper tool -- a snipper tool
-- geometry: a geometry object for a snip screenshot -- geometry [gears geometry]: a geometry object for a snip screenshot
-- --
-- @constructorfct awful.screenshot.new -- @constructorfct awful.screenshot.new
-- @tparam[opt] table args Table with the constructor parameters -- @tparam[opt] table args Table with the constructor parameters
@ -823,61 +821,60 @@ local function new(_, args)
enable_properties = true enable_properties = true
}) })
dir, prfx = configure_path(args.directory, args.prefix) local directory, prefix = configure_path(args.directory, args.prefix)
local fclr = nil local method = nil
if args.frame_color then local method_name = ""
fclr = gears.color(args.frame_color)
if not fclr then
fclr = frame_color
end
else
fclr = frame_color
end
local mthd = nil
local mthd_name = ""
if screenshot_methods[args.method] then if screenshot_methods[args.method] then
mthd = screenshot_methods[args.method] method = screenshot_methods[args.method]
mthd_name = args.method method_name = args.method
else else
mthd = screenshot_methods.default method = screenshot_methods.default
mthd_name = default_method_name method_name = default_method_name
end end
local scrn, clnt, mg_cb, geom local screen, client, on_success_cb, frame_color, geometry
if mthd_name == "screen" then if method_name == "screen" then
scrn = args.screen screen = args.screen
elseif mthd_name == "client" then elseif method_name == "client" then
clnt = args.client client = args.client
elseif mthd_name == "snipper" then elseif method_name == "snipper" then
mg_cb = args.on_success_cb
elseif mthd_name == "snip" then if args.frame_color then
frame_color = gears.color(args.frame_color)
if not frame_color then
frame_color = module_default_frame_color
end
else
frame_color = module_default_frame_color
end
if type(args.on_success_cb) == "function" then
on_success_cb = args.on_success_cb
else
on_success_cb = default_on_success_cb
end
elseif method_name == "snip" then
geom = args.geometry geom = args.geometry
end end
if type(args.on_success_cb) == "function" then
mg_cb = args.on_success_cb
else
mg_cb = default_on_success_cb
end
ss._private = { ss._private = {
directory = dir, directory = directory,
prefix = prfx, prefix = prefix,
filepath = nil, filepath = nil,
method_name = mthd_name, method_name = method_name,
frame_color = fclr, frame_color = frame_color,
screen = scrn, screen = screen,
client = clnt, client = client,
on_success_cb = mg_cb, on_success_cb = on_success_cb,
geometry = geom, geometry = geometry,
surface = nil surface = nil
} }
gears.table.crush(ss, screenshot, true) gears.table.crush(ss, screenshot, true)
return mthd(ss) return method(ss)
end end

View File

@ -11,7 +11,6 @@ local gdk = lgi.require('Gdk', '3.0')
local capi = { local capi = {
root = _G.root root = _G.root
} }
-- Dummy blue client for the client.content test -- Dummy blue client for the client.content test
-- the lua_executable portion may need to get ironed out. I need to specify 5.3 -- the lua_executable portion may need to get ironed out. I need to specify 5.3
local lua_executable = os.getenv("LUA") local lua_executable = os.getenv("LUA")
@ -34,6 +33,10 @@ Gtk:main{...}
local tiny_client = { lua_executable, "-e", string.format( local tiny_client = { lua_executable, "-e", string.format(
tiny_client_code_template, client_dim, client_dim)} tiny_client_code_template, client_dim, client_dim)}
-- We need a directory to configure the screenshot library to use even through
-- we never actually write a file.
local fake_screenshot_dir = "/tmp"
-- Split in the screen into 2 distict screens. -- Split in the screen into 2 distict screens.
screen[1]:split() screen[1]:split()
@ -107,7 +110,7 @@ end
local steps = {} local steps = {}
-- Check the whole root window. -- Check the whole root window with root.content()
table.insert(steps, function() table.insert(steps, function()
local root_width, root_height = root.size() local root_width, root_height = root.size()
local img = copy_to_image_surface(capi.root.content(), root_width, local img = copy_to_image_surface(capi.root.content(), root_width,
@ -128,17 +131,24 @@ table.insert(steps, function()
return true return true
end) end)
-- Check the screen.content -- Check screen.content
table.insert(steps, function() table.insert(steps, function()
for s in screen do for s in screen do
local geo = s.geometry local geo = s.geometry
local img = copy_to_image_surface(s.content, geo.width, geo.height) local img = copy_to_image_surface(s.content, geo.width, geo.height)
assert(get_pixel(img, 4, 4) == "#ff0000")
assert(get_pixel(img, geo.width - 4, 4) == "#ff0000")
assert(get_pixel(img, 4, geo.height - 4) == "#ff0000")
assert(get_pixel(img, geo.width - 4, geo.height - 4) == "#ff0000")
--[[
assert(get_pixel(img, geo.x + 4, geo.y + 4) == "#ff0000") assert(get_pixel(img, geo.x + 4, geo.y + 4) == "#ff0000")
assert(get_pixel(img, geo.x + geo.width - 4, geo.y + 4) == "#ff0000") assert(get_pixel(img, geo.x + geo.width - 4, geo.y + 4) == "#ff0000")
assert(get_pixel(img, geo.x + 4, geo.y + geo.height - 4) == "#ff0000") assert(get_pixel(img, geo.x + 4, geo.y + geo.height - 4) == "#ff0000")
assert(get_pixel(img, geo.x + geo.width - 4, geo.y + geo.height - 4) == "#ff0000") assert(get_pixel(img, geo.x + geo.width - 4, geo.y + geo.height - 4) == "#ff0000")
--]]
end end
@ -149,7 +159,7 @@ table.insert(steps, function()
return true return true
end) end)
-- Check the client.content -- Check client.content
table.insert(steps, function() table.insert(steps, function()
if #client.get() ~= 1 then return end if #client.get() ~= 1 then return end
local c = client.get()[1] local c = client.get()[1]
@ -168,11 +178,13 @@ table.insert(steps, function()
return true return true
end) end)
--Check root window with awful.screenshot module --Check the root window with awful.screenshot.root() method
table.insert(steps, function() table.insert(steps, function()
--Make sure client from last test is gone --Make sure client from last test is gone
if #client.get() ~= 0 then return end if #client.get() ~= 0 then return end
local ss = awful.screenshot.root()
local root_width, root_height = root.size()
local ss = awful.screenshot.root({directory = fake_screenshot_dir})
local img = ss.surface local img = ss.surface
if get_pixel(img, 100, 100) ~= "#00ff00" then return end if get_pixel(img, 100, 100) ~= "#00ff00" then return end
@ -190,17 +202,18 @@ table.insert(steps, function()
return true return true
end) end)
-- Check the screen.content -- Check the awful.screenshot.screen() method
table.insert(steps, function() table.insert(steps, function()
for s in screen do for s in screen do
local ss = awful.screenshot.screen({screen = s}) local geo = s.geometry
local ss = awful.screenshot.screen({screen = s, directory = fake_screenshot_dir})
local img = ss.surface local img = ss.surface
assert(get_pixel(img, geo.x + 4, geo.y + 4) == "#ff0000") assert(get_pixel(img, 4, 4) == "#ff0000")
assert(get_pixel(img, geo.x + geo.width - 4, geo.y + 4) == "#ff0000") assert(get_pixel(img, geo.width - 4, 4) == "#ff0000")
assert(get_pixel(img, geo.x + 4, geo.y + geo.height - 4) == "#ff0000") assert(get_pixel(img, 4, geo.height - 4) == "#ff0000")
assert(get_pixel(img, geo.x + geo.width - 4, geo.y + geo.height - 4) == "#ff0000") assert(get_pixel(img, geo.width - 4, geo.height - 4) == "#ff0000")
end end
@ -211,13 +224,14 @@ table.insert(steps, function()
return true return true
end) end)
-- Check the client.content -- Check the awful.screenshot.client() method
table.insert(steps, function() table.insert(steps, function()
if #client.get() ~= 1 then return end if #client.get() ~= 1 then return end
local c = client.get()[1] local c = client.get()[1]
local ss = awful.screenshot.client({client = c}) local geo = c:geometry()
local ss = awful.screenshot.client({client = c, directory = fake_screenshot_dir})
local img = ss.surface local img = ss.surface
if get_pixel(img, math.floor(geo.width / 2), math.floor(geo.height / 2)) ~= "#0000ff" then if get_pixel(img, math.floor(geo.width / 2), math.floor(geo.height / 2)) ~= "#0000ff" then