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

This allows matching a display by name or UUID -- helpful if two screens with the same name #1801

Closed
wants to merge 5 commits into from

Conversation

clarsen
Copy link

@clarsen clarsen commented May 26, 2018

No description provided.

@@ -10,6 +10,7 @@ local fnutils = require("hs.fnutils")
local screen = require("hs.screen")
local window = require("hs.window")
local application = require("hs.application")
local spi = require("hs._asm.undocumented.spaces.internal")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hs._asm.undocumented.spaces.internal module isn't shipped with Hammerspoon, so we can't include it in one of our modules.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I can revise so that caller should pass in an actually screen previously determined by hs._asm.undocumented.spaces.internal.UUIDforScreen that can be vetted by the existing code? I.e. no change really.

In other notes, I have a preprocessing enhancement for use of hs.layout that can deal with multi-monitors with layout specified like the example form below. Perhaps I should submit a separate issue to discuss possibly incorporating this into hs.layout?

oneMonitorDef = {
  {
    screen = "Color LCD";
    display_uuid = "A1067B4B-926A-055F-C929-DD7875BE07A3";
  },
}

twoMonitorDef  = {
  {
    screen = "Color LCD";
    display_uuid = "A1067B4B-926A-055F-C929-DD7875BE07A3";
  },
  {
    screen = "DELL U3011";
    display_uuid = "61346AEB-0959-1BE6-A8BE-36B9E98B97FC";
  },
}

windows = {
  {
    app = "Atom",
    title = ".*~/lsrc/data.and.reviews"
    oneMonitor = {
      geom = geoms.right75,
      screen_num = 1,
      space = 3
    },
    twoMonitor = {
      geom = geoms.right50,
      screen_num = 2,
      space = 1
    },
  },
...
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW it looks like the UUID is officially moving to being private API in 10.14. I'm also not convinced that it's properly unique - e.g. is the UUID stable across connections of the monitor?

My overall approach for hs.layout has always been to identify the monitors ahead of time - e.g. with two identical monitors I'd get hs.screen.primaryScreen() and then use :toEast() and :toWest() to determine which monitor is which, and pass them to hs.layout as leftMonitor and rightMonitor.

@@ -151,11 +152,26 @@ function layout.apply(layout, windowTitleComparator)
-- Find the destination display, if wanted
if _row[3] then
if type(_row[3]) == "string" then
local displays = fnutils.filter(screen.allScreens(), function(aScreen) return aScreen:name() == _row[3] end)
-- first try match as UUID

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W621) inconsistent indentation (SPACE followed by TAB)

@@ -151,11 +152,26 @@ function layout.apply(layout, windowTitleComparator)
-- Find the destination display, if wanted
if _row[3] then
if type(_row[3]) == "string" then
local displays = fnutils.filter(screen.allScreens(), function(aScreen) return aScreen:name() == _row[3] end)
-- first try match as UUID
local displays = fnutils.filter(screen.allScreens(), function(aScreen) return spi.UUIDforScreen(aScreen) == _row[3] end)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W621) inconsistent indentation (SPACE followed by TAB)

if displays then
-- TODO: This is bogus, multiple identical monitors will be impossible to lay out
display = displays[1]
display = displays[1]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W621) inconsistent indentation (SPACE followed by TAB)

-- TODO: This is bogus, multiple identical monitors will be impossible to lay out
display = displays[1]
display = displays[1]
else

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W621) inconsistent indentation (SPACE followed by TAB)

display = displays[1]
display = displays[1]
else
-- then try match as screen name

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W621) inconsistent indentation (SPACE followed by TAB)

end
-- check if it matches a UUID instead
if display == nil then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W621) inconsistent indentation (SPACE followed by TAB)

end
-- check if it matches a UUID instead
if display == nil then
for _, screen in pairs(hs.screen.allScreens()) do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W431) shadowing upvalue 'screen' on line 10

if display == nil then
for _, screen in pairs(hs.screen.allScreens()) do
if _row[3] == spi.UUIDforScreen(screen) then
display = screen

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W621) inconsistent indentation (SPACE followed by TAB)

if _row[3] == spi.UUIDforScreen(screen) then
display = screen
end
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W621) inconsistent indentation (SPACE followed by TAB)

display = screen
end
end
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(W621) inconsistent indentation (SPACE followed by TAB)

@cmsj
Copy link
Member

cmsj commented Aug 21, 2019

I'm gonna close this out, since it seems like there's no easy way forward.

@cmsj cmsj closed this Aug 21, 2019
@asmagill
Copy link
Member

@cmsj, where did you come across the info regarding the uuid api becoming private in 10.14? If there is a list of expected deprecations available, it would be useful for me to review it closely as well.

And FWIW, if the only code being used from the spaces module is to get the screen's uuid, that specific code actually doesn't use any of the private API functions leveraged for that module --

static int screenUUID(lua_State *L) {
    [[LuaSkin shared] checkArgs:LS_TUSERDATA, USERDATA_TAG, LS_TBREAK] ;

    NSScreen* screen = get_screen_arg(L, 1);
    CGDirectDisplayID cgID = [[[screen deviceDescription] objectForKey:@"NSScreenNumber"] unsignedIntValue] ;

    CFUUIDRef   theUUID    = CGDisplayCreateUUIDFromDisplayID(cgID) ;
    if (theUUID) {
        CFStringRef UUIDString = CFUUIDCreateString(kCFAllocatorDefault, theUUID) ;
        [[LuaSkin shared] pushNSObject:(__bridge_transfer NSString *)UUIDString] ;
        CFRelease(theUUID) ;
    } else {
        lua_pushnil(L) ;
    }
    return 1 ;
}

Not sure if that changes anything but thought I'd point it out.

@cmsj cmsj reopened this Aug 21, 2019
cmsj added a commit that referenced this pull request Aug 21, 2019
@cmsj
Copy link
Member

cmsj commented Aug 21, 2019

huh, I think I noticed CGDisplayCreateUUIDFromDisplayID moving from one framework to another and misread that as a deprecation/removal. I've just added hs.screen:getUUID() which should help :)

@asmagill
Copy link
Member

What is the status of this pull? hs.screen:getUUID() has been added, as noted above, so getting the UUID is now doable. What else is required?

@latenitefilms
Copy link
Contributor

@clarsen - Do you have any interest in updating this code to use hs.screen:getUUID() (as opposed to hs._asm.undocumented.spaces.internal) or would you like me to give it a crack in another pull request? Either way, it would be good to close off this pull request, as it's been sitting here for quite some time now. Thanks!

@asmagill asmagill closed this in 8ec10bf Sep 9, 2020
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

Successfully merging this pull request may close these issues.

5 participants