Merge pull request #2632 from Sorky/patch-1

Aligning 'regex' with other PR / Minor readability improvements

lib/menubar/utils.lua

@@ -127,8 +127,8 @@ local all_icon_sizes = {
     '16x16'
 }
 
---- List of supported icon formats.
-local supported_icon_formats = { png = 1, xpm = 2, svg = 3 }
+--- List of supported icon exts.
+local supported_icon_file_exts = { png = 1, xpm = 2, svg = 3 }
 
 local icon_lookup_path = nil
 --- Get a list of icon lookup paths.
@@ -214,29 +214,28 @@ function utils.lookup_icon_uncached(icon_file)
         return false
     end
 
-    local icon_file_ext = icon_file:match(".*%.(.-)$")
-    if icon_file:sub(1, 1) == '/' and supported_icon_formats[icon_file_ext] then
-        -- If the path to the icon is absolute and its format is
-        -- supported, do not perform a lookup.
+    local icon_file_ext = icon_file:match(".+%.(.*)$")
+    if icon_file:sub(1, 1) == '/' and supported_icon_file_exts[icon_file_ext] then
+        -- If the path to the icon is absolute do not perform a lookup [nil if unsupported ext or missing]
         return gfs.file_readable(icon_file) and icon_file or nil
     else
+        -- Look for the requested file in the lookup path
         for _, directory in ipairs(get_icon_lookup_path()) do
-            local directory_file = directory .. "/" .. icon_file
-            if supported_icon_formats[icon_file_ext] and
-                    gfs.file_readable(directory_file) then
-                return directory_file
+            local possible_file = directory .. "/" .. icon_file
+            -- Check to see if file exists if requested with a valid extension
+            if supported_icon_file_exts[icon_file_ext] and gfs.file_readable(possible_file) then
+                return possible_file
             else
-                -- Icon is probably specified without path and format,
-                -- like 'firefox'. Try to add supported extensions to
-                -- it and see if such file exists.
-                for format, _ in pairs(supported_icon_formats) do
-                    local possible_file = directory_file .. "." .. format
-                    if gfs.file_readable(possible_file) then
-                        return possible_file
+                -- Find files with any supported extension if icon specified without, eg: 'firefox'
+                for ext, _ in pairs(supported_icon_file_exts) do
+                    local possible_file_new_ext = possible_file .. "." .. ext
+                    if gfs.file_readable(possible_file_new_ext) then
+                        return possible_file_new_ext
                     end
                 end
             end
         end
+        -- No icon found
         return false
     end
 end

spec/menubar/usr/share/.filename.png

@@ -0,0 +1 @@
+../../../../icons/awesome16.png

spec/menubar/usr/share/icons/.filename.png

@@ -0,0 +1 @@
+../../../icons/fallback.png

spec/menubar/utils_spec.lua

@@ -76,60 +76,69 @@ describe("menubar.utils lookup_icon_uncached", function()
         assert_found_in_path('awesome2', '/.icons/awesome/scalable/apps/awesome2.png')
     end)
 
--- Check for no argument :: if not icon_file or icon_file == "" then
+    -- No argument
     it('no icon specified', function()
         assert.is_false(utils.lookup_icon_uncached())
-        assert.is_false(utils.lookup_icon_uncached(nil)) -- Same as above?
+        assert.is_false(utils.lookup_icon_uncached(nil))
         assert.is_false(utils.lookup_icon_uncached(false))
         assert.is_false(utils.lookup_icon_uncached(''))
     end)
 
--- Check for icon not in search path :: if icon_file:sub(1, 1) == '/' and supported_icon_formats[icon_file_ext] then
-    it('finds icons even those not in the search paths when full path specified', function()
+    -- Full path and filename with expected extension
+    it('finds icons even those not in the search paths when full path is specified', function()
 
         -- Shimmed icon base directories contain the following icons:
         --
         --     usr/share/icon5.png
         --     usr/share/icon6.xpm
         --     usr/share/icon7.svg
+        --     usr/share/icons/.filename.png
 
         assert_found_in_path('/usr/share/icon5.png', '/usr/share/icon5.png')
         assert_found_in_path('/usr/share/icon6.xpm', '/usr/share/icon6.xpm')
         assert_found_in_path('/usr/share/icon7.svg', '/usr/share/icon7.svg')
 
-        assert.is_same(nil, utils.lookup_icon_uncached('/.png')) -- supported file does not exist in location
-        assert.is_same(nil, utils.lookup_icon_uncached('/blah/icon6.png')) -- supported file does not exist in location
+        assert_found_in_path('/usr/share/.filename.png', '/usr/share/.filename.png')
+
+        assert.is_nil(utils.lookup_icon_uncached('/blah/icon6.png')) -- supported file does not exist in location
+        assert.is_nil(utils.lookup_icon_uncached('/.png')) -- path & supported ext - but also not found
     end)
 
--- Check icon with specified extension matching :: if supported_icon_formats[icon_file_ext] and
-    it('finds icons with specified supported extension in search path', function()
+   -- Filename with supported extension
+    it('finds icons with specified supported extension in the search path', function()
 
         -- Shimmed icon base directories contain the following icons:
         --
         --     .icons/icon5.png
         --     .icons/icon6.xpm
         --     .icons/icon7.svg
+        --     usr/share/icons/.filename.png
 
         assert_found_in_path('icon5.png', '/.icons/icon5.png')
         assert_found_in_path('icon6.xpm', '/.icons/icon6.xpm')
         assert_found_in_path('icon7.svg', '/.icons/icon7.svg')
 
+        assert_found_in_path('.filename.png', '/usr/share/icons/.filename.png')
+
         --  Will fail as file does not exist (but extension is supported)
         assert.is_false(utils.lookup_icon_uncached('icon8.png'))
     end)
 
--- Check icon with no (or invalid) extension matching :: if NOT supported_icon_formats[icon_file_ext]
+    -- Find icon with no (or invalid) extension matching
     it('finds icons without specified path or extension', function()
 
         -- Shimmed icon base directories contain the following icons:
         --
         --     .icons/icon6.xpm
         --     .icons/icon7.svg
+        --     usr/share/icons/.filename.png
 
-        assert_found_in_path('icon6', '/.icons/icon6.xpm') -- Similar to original tests and testing xpm extension
-        assert_found_in_path('icon7', '/.icons/icon7.svg') -- Similar to original tests and testing svg extension
+        assert_found_in_path('icon6', '/.icons/icon6.xpm')
+        assert_found_in_path('icon7', '/.icons/icon7.svg')
 
-        assert.is_false(utils.lookup_icon_uncached('/png')) -- supported file does not exist in given location
+        assert_found_in_path('.filename', '/usr/share/icons/.filename.png')
+
+        assert.is_false(utils.lookup_icon_uncached('/png')) -- path & no ext
         assert.is_false(utils.lookup_icon_uncached('.png')) -- file does not exist
         assert.is_false(utils.lookup_icon_uncached('png')) -- file does not exist
         assert.is_false(utils.lookup_icon_uncached('icon9')) -- file does not exist