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

Some ideas. #13

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

Some ideas. #13

wants to merge 10 commits into from

Conversation

aeghn
Copy link

@aeghn aeghn commented Feb 9, 2020

Cool script! Here are some ideas I finished.

  1. Select new font without rebooting fzf;
  2. Edit preview text on the fly (ctrl-t);
  3. Fix: Reboot sxiv if it is closed.

@sdushantha
Copy link
Owner

Could you explain what coproc is and how it works? I like to know how the code works before merging it :)

@aeghn
Copy link
Author

aeghn commented Feb 9, 2020

You can learn how coproc works in this page.

And I add some annotation to explain it.

@sdushantha sdushantha mentioned this pull request Feb 9, 2020
@aeghn
Copy link
Author

aeghn commented Feb 10, 2020

p-200210_114422

Hit Ctrl-h to see this help.

@aeghn
Copy link
Author

aeghn commented Feb 10, 2020

I have finished what I need. If you want to merge these commits, I wiil resolve the conflicts. :)

@aeghn
Copy link
Author

aeghn commented Feb 10, 2020

p-200210_165533

  1. Support PREVIEW_TEXT history now (saved to ~/.cache/fontpreview/preview_text)
  2. In preview text page:
    • Ctrl-t abort
    • Ctrl-s: accept selected items
    • ENTER: just print query
  3. Make help page a little useful by adding extra_menu to it.

@sdushantha
Copy link
Owner

@aeghn That looks great!
Is it by any chance possible to have the help menu in the same window? Or is that something fzf is not capable of doing?

@aeghn
Copy link
Author

aeghn commented Feb 10, 2020

I don't think to add everything to one page is a good thing. It will make that page messy.

Or could you make a make a diagram to explain ? @sdushantha

@sdushantha
Copy link
Owner

@aeghn Now I get what you did. I thought that the help menu was in a separate terminal window. I just read the code now and realized that in your screenshot, the you have the help menu in a separate terminal window just for the displaying purpose.

Just so you know, when you press [CTRL+S], when you are selecting the preview text from the history, the texts with \n wont show fully.
image

@aeghn
Copy link
Author

aeghn commented Feb 10, 2020

@sdushantha I'd like to open a new fzf window to show preview text page or help which just cover the old fzf window without change its status.

And I just fixed the bug you mentioned above. If you find any other bugs please tell me.

@sdushantha
Copy link
Owner

Could you put the font info on the right side? And display the help menu in the same way as the font info instead of starting a new fzf session? This way everything would look cleaner :)

Thank you for helping out a lot! After we merge this PR fontviewer will be really useful for people!!

@aeghn
Copy link
Author

aeghn commented Feb 11, 2020

preview

I tried to use a file to store the flag of whether showing help to achiveve this.

@sdushantha
Copy link
Owner

Everything seems to work well except one small thing...

When launching fontviewer it opens with the font info. It would be better to allow users to toggle it afterwards. At the moment, I can take it away by hitting CTRL+i. After doing so, when I hit CTRL+h to show the help, nothing happens. I have to first enable the font info and then hit CTRL+h in order to display the help menu.

After the fixing the tiny issue I mentioned above, I will merge it :)

@aeghn
Copy link
Author

aeghn commented Feb 11, 2020

It would be better to allow users to toggle it afterwards.

Actually fzf provided an arg to hide preview window by default. But I learned that you want to display pictures directly in fzf preview window, so I think it's okay to show preview by default.

After doing so, when I hit CTRL+h to show the help, nothing happens.

The result is as expected, fzf refresh its preview by toggle-preview twice. And fzf didn't provide an action just to open preview, besides, we couldn't get whether preview‘s status except to see by eyes. So, it's almost impossible except fzf add those one day.

But we can make a fake toggle action by modifying fzf_preview, like this:

Peek 2020-02-11 16-48

That almost succeeded except for the annoying border :(

@sdushantha
Copy link
Owner

That almost succeeded except for the annoying border :(

How did you did you initially create the info window then? Because in the last commit, you had the info menu/window shown and it would go away when you pressed CTRL+i again.
Hope that made sense

@aeghn
Copy link
Author

aeghn commented Feb 12, 2020

Because I think that was a failed attempt so I didn't submit it.

In my opinion, if you want to show font info and help message in one window and switch them by shortcuts, it's better to make preview window persist. Or add the option below the font, but this still need you to open preview window manually.

Peek 2020-02-12 09-48

Btw, what do you think of the help and information window now?

@sdushantha
Copy link
Owner

I made some minor changes from commit fef95fe: https://0x0.st/i-Oq.sh

I think it seems better now. Could you check it out and let me know what you think about it?

Also, Im a little confused with the code you wrote because Im not at your level of bash scripting, so just a small heads up that I might ask how something works in the future if I want to tweak it 👍

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.

2 participants