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

[Feature Request] Distinguish between monitors of the same model #81

Open
aidanlw505 opened this issue Jul 8, 2024 · 10 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@aidanlw505
Copy link

Describe the feature you'd like
Currently, if all of your monitors are of the same model, they have the exact same name. This makes it hard to know which one you're changing the layout for.

image

I think taking the monitor's number that's shown in display settings and including it in the title would be really helpful.

@domferr domferr added the enhancement New feature or request label Jul 8, 2024
@domferr
Copy link
Owner

domferr commented Jul 8, 2024

Hey @aidanlw505 thank you for sharing the issue and a possible solution! I agree with you. This will be fixed in the way you suggested by the next update

@domferr domferr added help wanted Extra attention is needed good first issue Good for newcomers labels Jul 8, 2024
@milhauzindahauz
Copy link

@domferr I would like to try to implement that.

@domferr
Copy link
Owner

domferr commented Jul 9, 2024

Hey @milhauzindahauz that's great!

The code to be changed is only in the file defaultMenu.ts. At line 172 there is the function _computeMonitorsDetails which runs a subprocess to retrieve the monitor's name.
The idea would be to:

  1. parse the monitor details got by the subprocess into JSON. This is already done at line 191:
const monitorsDetails = JSON.parse(stdout);

where monitorsDetails is a constant array. Each element of the array has type { name: string, x: number, y: number, height: number, width: number }

  1. rename the monitors that have the same name by appending to it the index of the monitor (since the monitor details are an array, it is enough to use the position in the array)

Please feel free to ask any question! I'm more than happy to provide any help 😄

Side notes:

  • use npm run build to clean and build
  • use npm run install:extension to copy the build into your local extensions folder. After logging out and then back in, the changes will take effect
  • if you don't want to log out to test your changes, you can run npm run dev:wayland to open a window with a nested GNOME Shell that has the changes you made to Tiling Shell. This is very useful since you can easily make a change, run the nested GNOME and see any error logged on the terminal you may have! However, since it is nested, that GNOME shell cannot see the other monitors.

@milhauzindahauz
Copy link

Thank for the guidance. But I got to the correct place by myself with the same idea. I found out that on X11 server names are null. I am trying to figure out how to obtain details on X11.

Further more:

public updateMonitorName(showMonitorName: boolean, monitorsDetails: { name: string, x: number, y: number, height: number, width: number }[]) {
if (!showMonitorName) this._label.hide();
else this._label.show();
const details = monitorsDetails.find(m => m.x === this._monitor.x && m.y === this._monitor.y);
if (!details) return;
this._label.set_text(details.name);
}

This method will update all monitors with same details of Monitor 1. Or am I missing anything?

@domferr
Copy link
Owner

domferr commented Jul 9, 2024

But I got to the correct place by myself with the same idea.

That's awesome!

I found out that on X11 server names are null. I am trying to figure out how to obtain details on X11.

Oh no, I didn't know that! Thank you!

This method will update all monitors with same details of Monitor 1. Or am I missing anything?

The DefaultMenu shows one LayoutsRow for each monitor (and it stores them into this._layoutsRows). For each monitor m, a LayoutsRow is created and the monitor m is passed to it. The updateMonitorName method is a member of the LayoutsRow class, so it will update the monitor name's label of only one LayoutsRow, based on the monitor m (this._monitor) passed via the constructor.

I'm not sure this answer to your question, let me know otherwise!

@milhauzindahauz
Copy link

The DefaultMenu shows one LayoutsRow for each monitor (and it stores them into this._layoutsRows). For each monitor m, a LayoutsRow is created and the monitor m is passed to it. The updateMonitorName method is a member of the LayoutsRow class, so it will update the monitor name's label of only one LayoutsRow, based on the monitor m (this._monitor) passed via the constructor.

This make sense. You are updating in by the details of the subprocess result. But lets have the situation as described above. @aidanlw505 has three monitors with same resolutions. If I would got array of the details for that I got three items in array and their resolutions are the same. And after that I try updateMonitorName by finding the same resolution three times. I should stop the finding on the first match that means first item with same resolution in the array => same detail (item at index 0) for all of them.

@GuillaumeAmat
Copy link

FWIW, the numbers attributed by GNOME to my monitors are not fixed at all 😅

I have 2 external screens, and they get numbers 2 and 3 interchangeably, each time I (un)plug my laptop.

It's probably fair to say that I should not be the only one experiencing this issue so, what do you think of the following addition?

  1. Open the Tiling Shell top bar menu
  2. Hover a layout on one of the monitors' line
  3. The related layout is drawn on the related monitor

That way, no matter the names/numbers are, you directly see where the layout is supposed to be used.

@domferr
Copy link
Owner

domferr commented Jul 9, 2024

The DefaultMenu shows one LayoutsRow for each monitor (and it stores them into this._layoutsRows). For each monitor m, a LayoutsRow is created and the monitor m is passed to it. The updateMonitorName method is a member of the LayoutsRow class, so it will update the monitor name's label of only one LayoutsRow, based on the monitor m (this._monitor) passed via the constructor.

This make sense. You are updating in by the details of the subprocess result. But lets have the situation as described above. @aidanlw505 has three monitors with same resolutions. If I would got array of the details for that I got three items in array and their resolutions are the same. And after that I try updateMonitorName by finding the same resolution three times. I should stop the finding on the first match that means first item with same resolution in the array => same detail (item at index 0) for all of them.

Yeah, I agree! I'd make the changes into the function _computeMonitorsDetails: after parsing the monitor details got by the subprocess into JSON (done at line 191), maybe it is a good idea to append the index to the monitor's name if there are other monitors with the same name.
A possible solution would be something like:

  1. create a map names that maps for each monitor's name an array of numbers, where the numbers are the indexes of the monitors in the monitorDetails array
  2. for each name in the map, if the associated array has multiple elements, you have the indexes of all the monitors that have the same name. For each index i, you can append i+1 to monitorDetails[i].name

I'm happy to know your thoughts about it! Feel free to implement any other idea or this one, as you prefer. I don't think we should care too much about time complexity because the monitors are going to be 2 or 3, at most 4 or 5 for very lucky people ahaha

@domferr
Copy link
Owner

domferr commented Jul 9, 2024

I have 2 external screens, and they get numbers 2 and 3 interchangeably, each time I (un)plug my laptop.

Hey @GuillaumeAmat, that's annoying ahaha 😢

The idea you are sharing is good imho. I would still keep a different name (something like LG Electronics 34" 1, LG Electronics 34" 2) for people who want to know as quick as possible which monitor that row is referring to.

@GuillaumeAmat
Copy link

The idea you are sharing is good imho. I would still keep a different name (something like LG Electronics 34" 1, LG Electronics 34" 2) for people who want to know as quick as possible which monitor that row is referring to.

Sure! That was just a suggestion to be added on top of what you already planned 🙏

milhauzindahauz pushed a commit to milhauzindahauz/tilingshell that referenced this issue Jul 10, 2024
@domferr domferr changed the title Distinguish between monitors of the same model [Feature Request] Distinguish between monitors of the same model Jul 14, 2024
milhauzindahauz added a commit to milhauzindahauz/tilingshell that referenced this issue Aug 1, 2024
milhauzindahauz added a commit to milhauzindahauz/tilingshell that referenced this issue Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants