Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

after update moving windows stopped working #2517

Closed
brando90 opened this issue Sep 25, 2020 · 17 comments
Closed

after update moving windows stopped working #2517

brando90 opened this issue Sep 25, 2020 · 17 comments

Comments

@brando90
Copy link

version: Version 0.9.80 (5530)

-- simple commands

hs.hotkey.bind({"cmd", "alt", "ctrl"}, "W", function()
    hs.alert.show("Hello World!")
    number_thirds_done = 0
  end)
  
  hs.hotkey.bind({"cmd", "alt", "ctrl"}, "R", function()
    hs.reload()
    number_thirds_done = 0
  end)
  hs.alert.show("Config loaded")
  
  hs.hotkey.bind({"cmd", "alt", "ctrl"}, "Space", function()
    local win = hs.window.focusedWindow()
    local f = win:frame()
    local screen = win:screen()
    local max = screen:frame()
  
    f.x = max.x
    f.y = max.y
    f.w = max.w
    f.h = max.h
    win:setFrame(f)
    number_thirds_done = 0
  end)
  
  -- 1/2 moves of windows
  
  hs.hotkey.bind({"cmd", "alt", "ctrl"}, "Left", function()
    local win = hs.window.focusedWindow()
    hs.console.printStyledtext(win)
    local f = win:frame()
    local screen = win:screen()
    local max = screen:frame()
  
    f.x = max.x
    --f.y = max.y
    f.w = max.w / 2
    --f.h = max.h
    win:setFrame(f)
    number_thirds_done = 0
  end)
  
  hs.hotkey.bind({"cmd", "alt", "ctrl"}, "Right", function()
    local win = hs.window.focusedWindow()
    local f = win:frame()
    local screen = win:screen()
    local max = screen:frame()
  
    f.x = max.x + (max.w / 2)
    --f.y = max.y
    f.w = max.w / 2
    --f.h = max.h 
    win:setFrame(f)
    number_thirds_done = 0
  end)
  
  hs.hotkey.bind({"cmd", "alt", "ctrl"}, "Up", function()
    local win = hs.window.focusedWindow()
    local f = win:frame()
    local screen = win:screen()
    local max = screen:frame()
  
    f.y = max.y
    f.h = max.h / 2
    win:setFrame(f)
    number_thirds_done = 0
  end)
  
  hs.hotkey.bind({"cmd", "alt", "ctrl"}, "Down", function()
    local win = hs.window.focusedWindow()
    local f = win:frame()
    local screen = win:screen()
    local max = screen:frame()
  
    f.h = max.h / 2
    f.y = max.y + (max.h / 2)
    win:setFrame(f)
    number_thirds_done = 0
  end)
  
  hs.hotkey.bind({"cmd", "alt"}, "Left", function()
    local win = hs.window.focusedWindow()
    win:moveOneScreenWest()
    number_thirds_done = 0
  end)
  
  hs.hotkey.bind({"cmd", "alt"}, "Right", function()
    local win = hs.window.focusedWindow()
    win:moveOneScreenEast()
    number_thirds_done = 0
  end)

  -- 1/3 moves of windows
  
  -- meant to track number of times we've done thirds. It's meant so that
  -- the first one gives 2/3 to the direction given and then another time gives 1/3
  -- doing it one more time resets everything and it starts splitting from the begininng (so it does not recursively split the previous size but instead from the max size)
  -- all other commands resets this flag to 0 so that splitting to thirds can be done
  number_thirds_done = 0

  hs.hotkey.bind({"alt", "ctrl"}, "Left", function()
    local win = hs.window.focusedWindow()
    hs.console.printStyledtext(win)
    local f = win:frame()
    local screen = win:screen()
    local max = screen:frame()
  
    if number_thirds_done == 0 then 
      f.x = max.x
      --f.y = max.y
      f.w = (2 / 3) * max.w 
      --f.h = max.h
      win:setFrame(f)
      number_thirds_done = 1
    elseif number_thirds_done == 1 then
      f.x = max.x
      --f.y = max.y
      f.w = (1 / 3) * max.w 
      --f.h = max.h
      win:setFrame(f)
      number_thirds_done = 2
    else
      print('done')
      number_thirds_done = 0
    end
  end)

  hs.hotkey.bind({"alt", "ctrl"}, "Right", function()
    local win = hs.window.focusedWindow()
    local f = win:frame()
    local screen = win:screen()
    local max = screen:frame()
  
    if number_thirds_done == 0 then
      f.x = max.x + (max.w / 3)
      --f.y = max.y
      f.w = (2/3) * max.w 
      --f.h = max.h
      win:setFrame(f)
      number_thirds_done = 1
    elseif number_thirds_done == 1 then
      f.x = max.x + ( (2/3) * max.w )
      --f.y = max.y
      f.w = (1/3) * max.w
      --f.h = max.h
      win:setFrame(f)
      number_thirds_done = 2
    else
      print('done')
      number_thirds_done = 0
    end
  end)

config.

@latenitefilms
Copy link
Contributor

This is a known issue, and will be fixed in the next release. In the meantime, you can build your own Hammerspoon version from the master branch.

@asmagill
Copy link
Member

What specific error message(s) are you receiving?

If the error is in response to : moveOneScreenWest or : moveOneScreenEast, then it's already been fixed in the master branch and will be in the next release. If the error is something else then it may be a new issue, which is why I need to know the specific error message you are getting.

@ylbeethoven
Copy link

WindowScreenLeftAndRight.spoon stopped working after update to Version 0.9.80 (5530).

The error message I'm getting is

2020-09-26 20:12:40: 20:12:40 ERROR: LuaSkin: hs.hotkey callback: ...mmerspoon/Spoons/WindowScreenLeftAndRight.spoon/init.lua:73: attempt to call a nil value (method 'moveOneScreenWest')
stack traceback:
...mmerspoon/Spoons/WindowScreenLeftAndRight.spoon/init.lua:73: in function 'WindowScreenLeftAndRight.moveCurrentWindowToScreen'
(...tail calls...)

@asmagill
Copy link
Member

These are indeed the exact issues address in the master branch and will be included in the new release when it is made available.

In the meantime, if you wish to manually apply the necessary fixes for this issue before the next release is built, you can apply the changes outlined in /~https://github.com/Hammerspoon/hammerspoon/pull/2488/files and 7a68956 directly by editing /Applications/Hammerspoon.app/Contents/Resources/extensions/hs/window/init.lua with any text editor of your choice.

I'm going to close this, but feel free to reopen if you think we've missed anything.

@brando90
Copy link
Author

This is a known issue, and will be fixed in the next release. In the meantime, you can build your own Hammerspoon version from the master branch.

how do I do this?

@brando90
Copy link
Author

This is a known issue, and will be fixed in the next release. In the meantime, you can build your own Hammerspoon version from the master branch.

perhaps we can create an issue so that hammer soon allows us to install whatever version we want?

@brando90
Copy link
Author

This is a known issue, and will be fixed in the next release. In the meantime, you can build your own Hammerspoon version from the master branch.

is there an issue that shows its being tracked?

@brando90
Copy link
Author

These are indeed the exact issues address in the master branch and will be included in the new release when it is made available.

In the meantime, if you wish to manually apply the necessary fixes for this issue before the next release is built, you can apply the changes outlined in /~https://github.com/Hammerspoon/hammerspoon/pull/2488/files and 7a68956 directly by editing /Applications/Hammerspoon.app/Contents/Resources/extensions/hs/window/init.lua with any text editor of your choice.

I'm going to close this, but feel free to reopen if you think we've missed anything.

It's not clear what I need to do with that code.

@brando90
Copy link
Author

: moveOneScreenEast

how do I test those to paste the error?

> : moveOneScreenWest
[string ": moveOneScreenWest"]:1: unexpected symbol near ':'

> : moveOneScreenEast
[string ": moveOneScreenEast"]:1: unexpected symbol near ':'

@brando90
Copy link
Author

This is a known issue, and will be fixed in the next release. In the meantime, you can build your own Hammerspoon version from the master branch.

when is the next release coming out?

@latenitefilms
Copy link
Contributor

Until a fix is released, the easiest option may be to just revert back to Hammerspoon 0.9.78.

/~https://github.com/Hammerspoon/hammerspoon/releases/tag/0.9.78

I would expect a new release relatively soon - at most within a week, but it just depends on the project maintainers availability.

@timriley
Copy link

timriley commented Sep 29, 2020

Hi folks, I've just upgraded to Version 0.9.81 (5594), and I'm also having errors moving windows.

Here are the errors I'm seeing:

2020-09-30 09:20:53: 09:20:53     hotkey: Disabled hotkey F16
2020-09-30 09:20:53:              hotkey: Disabled previous hotkey ⌃S
2020-09-30 09:20:55: 09:20:55     hotkey: Re-enabled previous hotkey ⌃S
2020-09-30 09:20:55:              hotkey: Enabled hotkey F16
2020-09-30 09:20:55: 09:20:55 ERROR:   LuaSkin: hs.hotkey callback: /Users/tim/.hammerspoon/windows.lua:261: attempt to call a nil value (field '?')
stack traceback:
	/Users/tim/.hammerspoon/windows.lua:261: in upvalue 'fn'
	/Users/tim/.hammerspoon/windows.lua:217: in function </Users/tim/.hammerspoon/windows.lua:215>

Here's the base config I'm using for moving windows:

windows.lua (originally from jasonrudolph/keyboard):

hs.window.animationDuration = 0

-- +-----------------+
-- |        |        |
-- |  HERE  |        |
-- |        |        |
-- +-----------------+
function hs.window.left(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:frame()

  f.x = max.x
  f.y = max.y
  f.w = max.w / 2
  f.h = max.h
  win:setFrame(f)
end

-- +-----------------+
-- |        |        |
-- |        |  HERE  |
-- |        |        |
-- +-----------------+
function hs.window.right(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:frame()

  f.x = max.x + (max.w / 2)
  f.y = max.y
  f.w = max.w / 2
  f.h = max.h
  win:setFrame(f)
end

-- +-----------------+
-- |      HERE       |
-- +-----------------+
-- |                 |
-- +-----------------+
function hs.window.up(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:frame()

  f.x = max.x
  f.w = max.w
  f.y = max.y
  f.h = max.h / 2
  win:setFrame(f)
end

-- +-----------------+
-- |                 |
-- +-----------------+
-- |      HERE       |
-- +-----------------+
function hs.window.down(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:frame()

  f.x = max.x
  f.w = max.w
  f.y = max.y + (max.h / 2)
  f.h = max.h / 2
  win:setFrame(f)
end

-- +-----------------+
-- |  HERE  |        |
-- +--------+        |
-- |                 |
-- +-----------------+
function hs.window.upLeft(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:fullFrame()

  f.x = max.x
  f.y = max.y
  f.w = max.w/2
  f.h = max.h/2
  win:setFrame(f)
end

-- +-----------------+
-- |                 |
-- +--------+        |
-- |  HERE  |        |
-- +-----------------+
function hs.window.downLeft(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:fullFrame()

  f.x = max.x
  f.y = max.y + (max.h / 2)
  f.w = max.w/2
  f.h = max.h/2
  win:setFrame(f)
end

-- +-----------------+
-- |                 |
-- |        +--------|
-- |        |  HERE  |
-- +-----------------+
function hs.window.downRight(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:fullFrame()

  f.x = max.x + (max.w / 2)
  f.y = max.y + (max.h / 2)
  f.w = max.w/2
  f.h = max.h/2

  win:setFrame(f)
end

-- +-----------------+
-- |        |  HERE  |
-- |        +--------|
-- |                 |
-- +-----------------+
function hs.window.upRight(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:fullFrame()

  f.x = max.x + (max.w / 2)
  f.y = max.y
  f.w = max.w/2
  f.h = max.h/2
  win:setFrame(f)
end

-- +--------------+
-- |  |        |  |
-- |  |  HERE  |  |
-- |  |        |  |
-- +---------------+
function hs.window.centerWithFullHeight(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:fullFrame()

  f.x = max.x + (max.w / 5)
  f.w = max.w * 3/5
  f.y = max.y
  f.h = max.h
  win:setFrame(f)
end

-- +-----------------+
-- |      |          |
-- | HERE |          |
-- |      |          |
-- +-----------------+
function hs.window.left40(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:frame()

  f.x = max.x
  f.y = max.y
  f.w = max.w * 0.4
  f.h = max.h
  win:setFrame(f)
end

-- +-----------------+
-- |      |          |
-- |      |   HERE   |
-- |      |          |
-- +-----------------+
function hs.window.right60(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:frame()

  f.x = max.w * 0.4
  f.y = max.y
  f.w = max.w * 0.6
  f.h = max.h
  win:setFrame(f)
end

function hs.window.nextScreen(win)
  local currentScreen = win:screen()
  local allScreens = hs.screen.allScreens()
  currentScreenIndex = hs.fnutils.indexOf(allScreens, currentScreen)
  nextScreenIndex = currentScreenIndex + 1

  if allScreens[nextScreenIndex] then
    win:moveToScreen(allScreens[nextScreenIndex])
  else
    win:moveToScreen(allScreens[1])
  end
end

windowLayoutMode = hs.hotkey.modal.new({}, 'F16')

windowLayoutMode.entered = function()
  windowLayoutMode.statusMessage:show()
end
windowLayoutMode.exited = function()
  windowLayoutMode.statusMessage:hide()
end

-- Bind the given key to call the given function and exit WindowLayout mode
function windowLayoutMode.bindWithAutomaticExit(mode, modifiers, key, fn)
  mode:bind(modifiers, key, function()
    mode:exit()
    fn()
  end)
end

local status, windowMappings = pcall(require, 'windows-bindings')

if not status then
  windowMappings = require('windows-bindings-defaults')
end

local modifiers = windowMappings.modifiers
local showHelp  = windowMappings.showHelp
local trigger   = windowMappings.trigger
local mappings  = windowMappings.mappings

function getModifiersStr(modifiers)
  local modMap = { shift = '', ctrl = '', alt = '', cmd = '' }
  local retVal = ''

  for i, v in ipairs(modifiers) do
    retVal = retVal .. modMap[v]
  end

  return retVal
end

local msgStr = getModifiersStr(modifiers)
msgStr = 'Window Layout Mode (' .. msgStr .. (string.len(msgStr) > 0 and '+' or '') .. trigger .. ')'

for i, mapping in ipairs(mappings) do
  local modifiers, trigger, winFunction = table.unpack(mapping)
  local hotKeyStr = getModifiersStr(modifiers)

  if showHelp == true then
    if string.len(hotKeyStr) > 0 then
      msgStr = msgStr .. (string.format('\n%10s+%s => %s', hotKeyStr, trigger, winFunction))
    else
      msgStr = msgStr .. (string.format('\n%11s => %s', trigger, winFunction))
    end
  end

  windowLayoutMode:bindWithAutomaticExit(modifiers, trigger, function()
    --example: hs.window.focusedWindow():upRight()
    local fw = hs.window.focusedWindow()
    fw[winFunction](fw)
  end)
end

local message = require('status-message')
windowLayoutMode.statusMessage = message.new(msgStr)

-- Use modifiers+trigger to toggle WindowLayout Mode
hs.hotkey.bind(modifiers, trigger, function()
  windowLayoutMode:enter()
end)
windowLayoutMode:bind(modifiers, trigger, function()
  windowLayoutMode:exit()
end)

windows-bindings-default.lua:

And the default bindings:

-- Default keybindings for WindowLayout Mode
--
-- To customize the key bindings for WindowLayout Mode, create a copy of this
-- file, save it as `windows-bindings.lua`, and edit the table below to
-- configure your preferred shortcuts.

--------------------------------------------------------------------------------
-- Define WindowLayout Mode
--
-- WindowLayout Mode allows you to manage window layout using keyboard shortcuts
-- that are on the home row, or very close to it. Use Control+s to turn
-- on WindowLayout mode. Then, use any shortcut below to perform a window layout
-- action. For example, to send the window left, press and release
-- Control+s, and then press h.
--
--   h/j/k/l => send window to the left/bottom/top/right half of the screen
--   i => send window to the upper left quarter of the screen
--   o => send window to the upper right quarter of the screen
--   , => send window to the lower left quarter of the screen
--   . => send window to the lower right quarter of the screen
--   return => make window full screen
--   n => send window to the next monitor
--   left => send window to the monitor on the left (if there is one)
--   right => send window to the monitor on the right (if there is one)
--------------------------------------------------------------------------------

return {
  modifiers = {'ctrl'},
  showHelp  = false,
  trigger   = 's',
  mappings  = {
    { {},         'return', 'maximize' },
    { {},         'space',  'centerWithFullHeight' },
    { {},         'h',      'left' },
    { {},         'j',      'down' },
    { {},         'k',      'up' },
    { {},         'l',      'right' },
    { {'shift'},  'h',      'left40' },
    { {'shift'},  'l',      'right60' },
    { {},         'i',      'upLeft' },
    { {},         'o',      'upRight' },
    { {},         ',',      'downLeft' },
    { {},         '.',      'downRight' },
    { {},         'n',      'nextScreen' },
    { {},         'right',  'moveOneScreenEast' },
    { {},         'left',   'moveOneScreenWest' },
  }
}

The particular error above came from triggering the right60 function, e.g.

function hs.window.right60(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:frame()

  f.x = max.w * 0.4
  f.y = max.y
  f.w = max.w * 0.6
  f.h = max.h
  win:setFrame(f)
end

But similar errors come from any of the functions that move windows, e.g. upRight, left40, centerWithFullHeight, etc.

Line 261 from windows.lua (i.e. the source of the error) is at the bottom of this snippet:

for i, mapping in ipairs(mappings) do
  local modifiers, trigger, winFunction = table.unpack(mapping)
  local hotKeyStr = getModifiersStr(modifiers)

  if showHelp == true then
    if string.len(hotKeyStr) > 0 then
      msgStr = msgStr .. (string.format('\n%10s+%s => %s', hotKeyStr, trigger, winFunction))
    else
      msgStr = msgStr .. (string.format('\n%11s => %s', trigger, winFunction))
    end
  end

  windowLayoutMode:bindWithAutomaticExit(modifiers, trigger, function()
    --example: hs.window.focusedWindow():upRight()
    local fw = hs.window.focusedWindow()
    fw[winFunction](fw)
     --- ^^^ THIS LINE ABOVE IS 261 ^^^
  end)
end

I'm adding this here because of the comment from @asmagill above:

If the error is in response to : moveOneScreenWest or : moveOneScreenEast, then it's already been fixed in the master branch and will be in the next release. If the error is something else then it may be a new issue, which is why I need to know the specific error message you are getting.

In this case, my config is very similar to the one from the original issue posting, but my error is not in response to the moveOneScreenWest, etc. functions.

Let me know if I should move this to a new issue, thanks!

@asmagill
Copy link
Member

@timriley what you have in window.lua worked in 0.9.78 and previous because hs.window used the same table for both functions and methods -- a design decision that we have been avoiding for as long as I can remember. With 0.9.79+ there has been a rewrite of hs.window, hs.uielement, and hs.application that, among other fixes and additions, separates functions (which are invoked directly from the module, e.g. hs.window.focusedWindow()) from methods (which are invoked directly on a hs.window object, e.g. in hs.window.focusedWindow():maximize() the "maximize()" is a method invoked on the object returned by "hs.window.focusedWindow()").

The proper fix would be not to modify the hs.window module at all as you run the risk that in the future we might add something under one of those names and something might break... but that entails a larger rewrite than I can easily describe in here. And the syntax of hs.window is probably pretty static at this point, so name collisions in the future are unlikely, though I must stress not impossible.

The "cleanest" simple fix, to maintain the separation described above, would be to rewrite the declaration to each of your additions to hs.window like this (showing only for left, but the same would need to be applied to all of them):

function hs.getObjectMetatable("hs.window").left(win)
  ... rest of function is unchanged ...
end

This causes the new "method" to be added to the correct table for objects so that fw[winFunction](fw) in line 261 will work.

Another approach that might be seen as "simpler" in the sense that it requires no modifications to any of your existing functions, is to modify the __index method of the window objects so that they look in the hs.window module if the method isn't defined in the methods table. Something like this at the very top of your window.lua file:

local objectMT = hs.getObjectMetatable("hs.window")
local original__index = objectMT.__index
objectMT.__index = function(_, key)
    return original__index[key] or hs.window[key]
end

This replaces the object method lookup table with a function that checks the original table for the method name and if one isn't found, then checks the hs.window function table.

@timriley
Copy link

Thanks for the amazingly explained advice, @asmagill!

FWIW I've just tried the first approach you recommended, e.g.

function hs.getObjectMetatable("hs.window").left(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:frame()

  f.x = max.x
  f.y = max.y
  f.w = max.w / 2
  f.h = max.h
  win:setFrame(f)
end

But it results in this error when loading the config:

2020-09-30 14:04:30: *** ERROR: error loading module 'windows' from file '/Users/tim/.hammerspoon/windows.lua':
	/Users/tim/.hammerspoon/windows.lua:8: <name> or '...' expected near '"hs.window"'
stack traceback:
	[C]: in ?
	[C]: in function 'rawrequire'
	...app/Contents/Resources/extensions/hs/_coresetup/init.lua:651: in function 'require'
	/Users/tim/.hammerspoon/init.lua:23: in main chunk
	[C]: in function 'xpcall'
	...app/Contents/Resources/extensions/hs/_coresetup/init.lua:702: in function 'hs._coresetup.setup'
	(...tail calls...)

I spelunked around some of Hammerspoon's own Lua code to look for similar usages, and found this pattern, which I tried:

local windowMT = hs.getObjectMetatable("hs.window")

function windowMT.left(win)
  local f = win:frame()
  local screen = win:screen()
  local max = screen:frame()

  f.x = max.x
  f.y = max.y
  f.w = max.w / 2
  f.h = max.h
  win:setFrame(f)
end

And that one worked!

So thanks for helping unblock me here, that was really helpful. But I'd also love to understand why one approach worked but not the other, if you happen to have a little more time. Thank you! 😄

@asmagill
Copy link
Member

Hmmm... odd, but I think I know why... it has to do with how you define your functions. For most use cases name = function(x) ... end and function name(x) ... end are equivalent, just a matter of stylistic preference and I happen to prefer the former and was thinking in that manner when I made my suggestion.

Your approach of putting the function keyword first must not like have executable code as part of the destination... while windowMT.left specifies a table and a key within that table, hs.getObjectMetatable("hs.window").left specifies executable code which will return a table and then applies left as a key to that returned table.

I suspect it would work if you had written hs.getObjectMetatable("hs.window").left = function(win) ... end.

Sorry for missing that nuance in syntax!

@timriley
Copy link

@asmagill no worries! Thank you again for your help here, I've learnt a lot through this exchange :)

@vesper8
Copy link

vesper8 commented Oct 13, 2020

this is still broken for me 0.9.81.. are we obliged to change our code or will there be a release that undoes the breaking change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants