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

Open selected result in quicklook #859

Closed
wants to merge 13 commits into from

Conversation

Garulf
Copy link
Member

@Garulf Garulf commented Dec 3, 2021

F1 Opens Quicklook on your selection.

Right now it's just a proof of concept.... Logic and features will be added soon™

Todo:

  • Change preview when selection changes
  • Logic to determine what to open
  • Add field for Plugin's to use
    ...and more

Preview:

2021-12-03.13-12-00.mp4

@Garulf Garulf self-assigned this Dec 3, 2021
@Garulf Garulf added the enhancement New feature or request label Dec 3, 2021
@Garulf Garulf added this to the Future milestone Dec 6, 2021
@Garulf Garulf modified the milestones: Future, 1.10.0 Jan 25, 2022
@Garulf Garulf marked this pull request as ready for review January 25, 2022 08:49
Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

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

I am getting freezing with any query I type. This seems to be the error:
System.Windows.Data Error: 23 : Cannot convert '' from type '' to type 'System.Windows.Media.ImageSource' for 'en-US' culture with default conversions; consider using Converter property of Binding. NotSupportedException:'System.NotSupportedException: ImageSourceConverter cannot convert from (null).
at System.ComponentModel.TypeConverter.GetConvertFromException(Object value)
at System.Windows.Media.ImageSourceConverter.ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, Object value)
at MS.Internal.Data.DefaultValueConverter.ConvertHelper(Object o, Type destinationType, DependencyObject targetElement, CultureInfo culture, Boolean isForward)'

@Garulf
Copy link
Member Author

Garulf commented Jan 27, 2022

Does it say what line is triggering this exception?

@jjw24
Copy link
Member

jjw24 commented Jan 27, 2022

Nah sorry, that's why I didn't dig into it, I will leave it with you since I want to continue onto other prs. You can replicate the issue right?

@Garulf
Copy link
Member Author

Garulf commented Jan 27, 2022

Nah sorry, that's why I didn't dig into it, I will leave it with you since I want to continue onto other prs. You can replicate the issue right?

I never got this error. Is this from the CI build?

@jjw24
Copy link
Member

jjw24 commented Jan 27, 2022

it's coming from local run in visual studio

@jjw24
Copy link
Member

jjw24 commented Jan 27, 2022

Tested with the CI build, it's happening there as well. Simply type 'a' or jpg causes flow to hang indefinitely.

@Garulf
Copy link
Member Author

Garulf commented Jan 28, 2022

Tested with the CI build, it's happening there as well. Simply type 'a' or jpg causes flow to hang indefinitely.

Found the issue!

If QuickLook is unavailable and Flow tries to open a pipe and gets stuck. Adding a timeout helps but isn't the full solution.

An Async option would be best.

@jjw24
Copy link
Member

jjw24 commented Jan 30, 2022

When would quick look not be available? Is it when the file is not an image?

@jjw24 jjw24 added the review in progress Indicates that a review is in progress for this PR label Jan 30, 2022
@jjw24
Copy link
Member

jjw24 commented Feb 2, 2022

so this needs quicklook installed before can be used? do we want to use Droplex to handle it?

@Garulf
Copy link
Member Author

Garulf commented Feb 2, 2022

so this needs quicklook installed before can be used? do we want to use Droplex to handle it?

I'm unsure. I originally designed this so it would just work no matter if it was installed or not.

@Flow-Launcher/team any opinions?

@jjw24
Copy link
Member

jjw24 commented Feb 3, 2022

I wonder if there is a way we can set QL to stay on top, hide and set window size via the code? This will help make QL usage feel more integrated with flow.

@Garulf
Copy link
Member Author

Garulf commented Feb 3, 2022

I wonder if there is a way we can set QL to stay on top, hide and set window size via the code? This will help make QL usage feel more integrated with flow.

As far as I can tell there is no API to pragmatically control QuickLook other than simply toggling it and switching files.

QL-Win/QuickLook#557

@jjw24
Copy link
Member

jjw24 commented Feb 8, 2022

TODO: need to check if window is open to avoid wasting CPU time.

@jjw24
Copy link
Member

jjw24 commented Feb 8, 2022

You can set QL to stay on top on each launch by setting QuickLook.config's attribute TopMost to true

@Garulf
Copy link
Member Author

Garulf commented Feb 8, 2022

TODO: need to check if window is open to avoid wasting CPU time.

Quicklook author seems to think detecting the window is more cpu time then just sending the pipe and failing. I haven't tested this but it seems logical.

@jjw24
Copy link
Member

jjw24 commented Feb 9, 2022

I got the impression from the linked issue the author meant better to check window exists so to avoid wasting CPU, where did you see sending the pipe and failing is better? If failing is better than yeah that would be good.

@Garulf
Copy link
Member Author

Garulf commented Feb 9, 2022

I got the impression from the linked issue the author meant better to check window exists so to avoid wasting CPU, where did you see sending the pipe and failing is better? If failing is better than yeah that would be good.

You’re correct. I must have misread his intentions.

@jjw24 jjw24 removed the review in progress Indicates that a review is in progress for this PR label Jul 17, 2022
@jjw24 jjw24 modified the milestones: 1.10.0, Future Dec 12, 2022
@Garulf
Copy link
Member Author

Garulf commented Apr 22, 2023

Closing in favor of #2082

@Garulf Garulf closed this Apr 22, 2023
@jjw24 jjw24 removed this from the Future milestone Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Proof of Concept
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants