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

[Strategy] solved focus issue #12

Closed
maxdevjs opened this issue Feb 8, 2020 · 9 comments
Closed

[Strategy] solved focus issue #12

maxdevjs opened this issue Feb 8, 2020 · 9 comments
Labels
enhancement New feature or request

Comments

@maxdevjs
Copy link
Contributor

maxdevjs commented Feb 8, 2020

Two points:

  • does not switch the focus back to the terminal
  • quitting sxiv, then fzf gets confused

I got the first working in terminal, but not yet in fontpreview.
The second will probalby need a review of opening/closing strategy.

Let me know :)

@maxdevjs maxdevjs changed the title [Strategy] [Strategy] solved focus issue Feb 8, 2020
@maxdevjs
Copy link
Contributor Author

maxdevjs commented Feb 8, 2020

Solved fist, now switches back to the terminal.

Please check and test it.

Must be cleaned, but that's it. If it is good for you, after cleaning I will send PR.

@sdushantha
Copy link
Owner

Well the sleep 0.5 seems to be no issue for me other than the small delay of course.

This is just a suggestion: could we maybe add a flag that could fix the focusing issue? Because not everyone has this issue so I guess it would be nice for them not to have the extra 0.5 sec delay.

Let me know what you think!

@sdushantha sdushantha added the enhancement New feature or request label Feb 9, 2020
@maxdevjs
Copy link
Contributor Author

maxdevjs commented Feb 9, 2020

Thoughts. Some are random.

The sleep is intended to be tamed, that's why I did not send a PR and left a higher value. 0.3 (or even less) can be an optimal value.

As I tested fontviewer in not so recent hardware, it is possible that newer systems are fast enough to not need it. Point is to have an average value that can work for everyone, even in old machines, without making the script too complicated. A minimal value would be imperceptible.

Also, in my tests windowfocus does not work. Only way to make it work has been windowactivate. Is your case different?

Test with: xdotool version 3.20191225.1

I have another install with xdotool version 3.20180830.1, but not tested yet.

A flag (is sxiv instance tied to fontviewer running?) would probably be interesting for the second case (fzf not responding if sxiv is quit), pre_exit should kill fzf, or something similar to clean state (ps returns that there is no sxiv then kill fzf - probably there are no multiple fzf instances running on system, but better to check for the specific one tied to fontviewer instance).

Edit: going to check #13

Also, #10 does not solve completely the /tmp crowding issue: exiting fontviewer could also clean temp directory every time (and/or several temp directories -several run of fontviewer- could be saved as subdirectories in an unique fontviewer temp directory (!)).

Please, let me know what are your preferences and how it works on your system.

@sdushantha
Copy link
Owner

@maxdevjs

A flag (is sxiv instance tied to fontviewer running?) would probably be interesting for the second case (fzf not responding if sxiv is quit), pre_exit should kill fzf, or something similar to clean state (ps returns that there is no sxiv then kill fzf - probably there are no multiple fzf instances running on system, but better to check for the specific one tied to fontviewer instance).

I did not really understand this...I think I will have to reread it tomorrow to understand it

Also, in my tests windowfocus does not work. Only way to make it work has been windowactivate. Is your case different?

windowactivate does not work for me, only windowfocus does


Also, #10 does not solve completely the /tmp crowding issue: exiting fontviewer could also clean temp directory every time (and/or several temp directories -several run of fontviewer- could be saved as subdirectories in an unique fontviewer temp directory (!)).

#19 Solves this. Now when fontpreview exits, it deletes the temp dir

@maxdevjs
Copy link
Contributor Author

maxdevjs commented Feb 9, 2020

@sdushantha

I did not really understand this...I think I will have to reread it tomorrow to understand it

Never mind :) It is probably addressed by #13 , but I do not understand completely what the code does. coproc seems to be a very cool add to the project.

windowactivate does not work for me, only windowfocus does

Nice :D What is your xdotool version? And distro? For what I know so far, I am the only one with this issue.

@sdushantha
Copy link
Owner

@maxdevjs

Never mind :) It is probably addressed by #13 , but I do not understand completely what the code does. coproc seems to be a very cool add to the project.

Yeah, but Ive got to read about coproc so I get an understanding on how it works because I dont want to end up having code in my script which I have no idea about.

Nice :D What is your xdotool version? And distro? For what I know so far, I am the only one with this issue.

$ xdotool -v
xdotool version 3.20160805.1

$ uname -a
Linux unicorn 5.4.17-1-MANJARO #1 SMP PREEMPT Tue Feb 4 11:40:50 UTC 2020 x86_64 GNU/Linux

@maxdevjs
Copy link
Contributor Author

maxdevjs commented Feb 10, 2020

3.2016: does Manjaro have later versions? If so, do you have a bit of time to install something like 3.2018 or 3.2019 to check if later versions introduced the behaviour I relate?

That could give you an idea if it is worth to update the script to support both kind of behaviour or not.

Also, on your system, do you need the sleep to get the terminal focused back or not?

@sdushantha
Copy link
Owner

sdushantha commented Feb 11, 2020

@maxdevjs

Also, on your system, do you need the sleep to get the terminal focused back or not?

No, I dont need sleep to get the terminal back to focus

I just installed the latest version of xdotool, and I dont have any issues with it:

$ xdotool -v                                                                                                                                                
xdotool version 3.+20200211.1

Sorry for the late reply, I had a lot of work to complete

@maxdevjs
Copy link
Contributor Author

@sdushantha

Interesting. And not problem, I closed this because has no relevance anymore :)

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

No branches or pull requests

2 participants