Skip to content
This repository has been archived by the owner on Dec 23, 2018. It is now read-only.

Toggle last desktop #65

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Toggle last desktop #65

wants to merge 5 commits into from

Conversation

GioBonvi
Copy link
Collaborator

I've tested this with all the combinations of programs, windows, and keybindings I could think of: it probably should be tested by someone else to confirm that everything works ok.
Maybe @robertmeta could help as he was interested in the feature.

This solves #40.

Giorgio Bonvicini and others added 4 commits May 17, 2017 00:16
This is a rapid jot down of the first ideas about enabling a hotkey
to toggle between the current desktop and the last one.

I am not really shure this is the most elegant way to do this, still,
it seems to work ;)
Removed an unnecessary if caused by mindless copy-pasting.

Fixed a bug in the management of the timing which could cause unexpected
behaviours in some edge cases.
@GioBonvi GioBonvi requested a review from sdias June 29, 2017 10:30
OnShiftLastActivePress() {
; Shift to the last active desktop.

global lastActiveDesktopNumber
Copy link
Owner

Choose a reason for hiding this comment

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

You shouldn't have to identify that the variable is global if you already did it before when you defined it at the top of the file, can you look into this to make it work like it does for all of the other globals?

@sdias
Copy link
Owner

sdias commented Jul 1, 2017

Two comments:

  • I'm not sure if it will be responsive enough if it has to wait one second... But I guess it's easy to change if get feedback on that.
  • You need to add checks to make sure that the previous desktop isn't defined to be the same as the current one, because then the hotkey does nothing. I would expect the last active desktop to not change in that situation:
    • Go to desktop 1
    • Wait 2 secs
    • Go to desktop 3
    • Wait 2 secs
    • Go to desktop 2
    • Wait 0.5 secs
    • Go to desktop 3
    • Wait 2 secs
    • Go to last desktop
    • Expected: Goes to desktop 1
    • Result: Goes to desktop 3 (does nothing).

@GioBonvi
Copy link
Collaborator Author

GioBonvi commented Jul 1, 2017

You are definitely right on the second one: I will try and fix it later.

Regarding the first issue: what would you propose to change it to? My only concern was that if the value was too low it would register as "last active" desktops while rolling through if the user was not fast enough.
Actually I also had thought about another mechanism: the user could manually "pin" two desktops and switch between those, like this:

  • pin 1
  • go to 3
  • pin 3

Now if you are on any desktop and issue the "switch to last" you go to 3, if you are on 3 you go to 1 and if you are on 1 you go to 3.

If you have already pinned two desktops and pin another one they would "shift":

  • pin 1 (Pinned 1)
  • go to 3
  • pin 3 (Pinned 1, 3)
  • go to 5
  • pin 5 (Pinned 3, 5)

However I discarded the idea beause it was more complicated and less intuitive. (Espcially withou any way to rapidly verify which desktops are currently pinned).

@sdias
Copy link
Owner

sdias commented Jul 1, 2017

I think it is good as is. I see your point. I think that when you switch to a specific desktop, it isn't a problem at all, and we don't even really need the delay, but when switching to the prev/next desktop, we do. I think it is a good enough solution for now.
I like your idea of pinning a desktop, but I agree with criticisms of it. Another similar thing we could do is to enable you to mark desktops as favorite in runtime (maybe config as well?) and enable you to switch to the prev/next favorite desktop; it would be pretty simple to understand and it would enable you to navigate between a subset of desktops.

@GioBonvi
Copy link
Collaborator Author

GioBonvi commented Jul 1, 2017

Another similar thing we could do is to enable you to mark desktops as favorite in runtime (maybe config as well?) and enable you to switch to the prev/next favorite desktop; it would be pretty simple to understand and it would enable you to navigate between a subset of desktops.

Love this implementation: having a subset of favorite desktops could definitely be useful and it would be much easier to manage.
You could open an issue with a brief description of the feature to collect some feedback and ideas.

@sdias
Copy link
Owner

sdias commented Jul 1, 2017

Love this implementation: having a subset of favorite desktops could definitely be useful and it would be much easier to manage.
You could open an issue with a brief description of the feature to collect some feedback and ideas.

Agreed. Feel free to merge this.
By the way, do you want to do the release yourself? All you need to do is:

  • Compile the AHK file using the right click menu on File Explorer
  • Create a ZIP file of the folder contents, and give it a name following the same pattern that the ones in the releases
  • Draft a new release on the GitHub page for the project
    • Bump the version (first number is for breaking changes, second number for new features, third for bug fixes); in this case just bump it to 0.11.0.
    • Set the tag version field to the release number
    • Set the release title to "Beta "
    • Identify the new features and bugfixes if applicable, in the same way that I did for past releases, and add other notes if necessary
    • If you disagree with any of this, we can change it :)

Would you mind doing the release?

@GioBonvi
Copy link
Collaborator Author

GioBonvi commented Jul 1, 2017

I'll do it: no problem, just two questions:

  1. the documentation in the release: should we convert it to HTML or keep the Markdown?
  2. source code uses Unix newline characters (LF or /n), while releases use Windows newline characters (CRLF or /r/n): how did you convert from one to the other?
  3. lastly whould I make two separated releases for the two merged PRs?

@sdias
Copy link
Owner

sdias commented Jul 1, 2017

  1. the documentation in the release: should we convert it to HTML or keep the Markdown?
    Markdown.

We've discussed this previously. I think we should use HTML for releases, but I don't want us to maintain both versions manually. We need a proper build script that converts the MD to HTML, and probably to handle other release tasks as well, but that's future work, so let's continue just using MD for now.

  1. source code uses Unix newline characters (LF or /n), while releases use Windows newline characters (CRLF or /r/n): how did you convert from one to the other?

I installed git on Windows and it pulls and converts to windows line terminators and converts it back to unix ones when it pushes. The repo code should only have unix line terminators.

  1. lastly whould I make two separated releases for the two merged PRs?

Make just one release and list all of the changes.

Also, one last detail: please address the comment that I left in the code for this branch related to that global variable you introduced.

@GioBonvi
Copy link
Collaborator Author

GioBonvi commented Jul 1, 2017

I've built the release and fixed the global variables.

I'll work on the main problems you have found when I'll have time as it looks like it won't be a five minute fix.

Thank you for your time and assitance, have a nice weekend!

@sdias
Copy link
Owner

sdias commented Jul 1, 2017

No worries, thank you so much for your contributions again. Great work as always :) Have a nice weekend. :)

@GioBonvi
Copy link
Collaborator Author

GioBonvi commented Jul 7, 2017

I am eperiencing some trouble implementing this correctly: it looks like it would involve a minimum of two elements stored as "history of previous active desktops" nad still I haven't figured it out correctly.

The more I work on this the more I think this is not the best way to realize it: I really think your "favourite desktops" feature would be much more useful and senseful.

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

Successfully merging this pull request may close these issues.

2 participants