-
-
Notifications
You must be signed in to change notification settings - Fork 616
Allow plugins to use web URL's for icon paths #1351
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
Changes from 13 commits
ffbd8fd
03e319e
fda155a
b5c2125
d62d710
301882d
5c33b0d
b7900b2
57d63cf
ed107cc
7f1b9d0
416c44f
ec1a061
ebd6f17
eb3f723
ef575bb
3708489
191c6af
957c4e2
eb5e33a
c39a727
99fd8d1
2507d1a
ba0aee1
cae0b7b
e9bf62e
0fd127a
cf9dd4a
ffa40b0
ff2ebc8
e86a2f1
28256a7
101593a
169857b
26668b4
ce52919
414e55c
622b130
e5948a7
67443fd
3665bd9
83ec809
303d3b9
fb3a23f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,7 +68,7 @@ public static void UpdateProxy(ProxyProperty property) | |
| var userName when string.IsNullOrEmpty(userName) => | ||
| (new Uri($"http://{Proxy.Server}:{Proxy.Port}"), null), | ||
| _ => (new Uri($"http://{Proxy.Server}:{Proxy.Port}"), | ||
| new NetworkCredential(Proxy.UserName, Proxy.Password)) | ||
| new NetworkCredential(Proxy.UserName, Proxy.Password)) | ||
| }, | ||
| _ => (null, null) | ||
| }, | ||
|
|
@@ -79,7 +79,7 @@ var userName when string.IsNullOrEmpty(userName) => | |
| _ => throw new ArgumentOutOfRangeException() | ||
| }; | ||
| } | ||
| catch(UriFormatException e) | ||
| catch (UriFormatException e) | ||
| { | ||
| API.ShowMsg("Please try again", "Unable to parse Http Proxy"); | ||
| Log.Exception("Flow.Launcher.Infrastructure.Http", "Unable to parse Uri", e); | ||
|
|
@@ -94,7 +94,7 @@ public static async Task DownloadAsync([NotNull] string url, [NotNull] string fi | |
| if (response.StatusCode == HttpStatusCode.OK) | ||
| { | ||
| await using var fileStream = new FileStream(filePath, FileMode.CreateNew); | ||
| await response.Content.CopyToAsync(fileStream); | ||
| await response.Content.CopyToAsync(fileStream, token); | ||
| } | ||
| else | ||
| { | ||
|
|
@@ -117,7 +117,7 @@ public static async Task DownloadAsync([NotNull] string url, [NotNull] string fi | |
| public static Task<string> GetAsync([NotNull] string url, CancellationToken token = default) | ||
| { | ||
| Log.Debug($"|Http.Get|Url <{url}>"); | ||
| return GetAsync(new Uri(url.Replace("#", "%23")), token); | ||
| return GetAsync(new Uri(url), token); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this change intended?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I don't think that's needed since it is creating a url.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you mean? the old code is also creating a url too no? |
||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -130,36 +130,57 @@ public static async Task<string> GetAsync([NotNull] Uri url, CancellationToken t | |
| { | ||
| Log.Debug($"|Http.Get|Url <{url}>"); | ||
| using var response = await client.GetAsync(url, token); | ||
| var content = await response.Content.ReadAsStringAsync(); | ||
| if (response.StatusCode == HttpStatusCode.OK) | ||
| { | ||
| return content; | ||
| } | ||
| else | ||
| var content = await response.Content.ReadAsStringAsync(token); | ||
| if (response.StatusCode != HttpStatusCode.OK) | ||
| { | ||
| throw new HttpRequestException( | ||
| $"Error code <{response.StatusCode}> with content <{content}> returned from <{url}>"); | ||
| } | ||
|
|
||
| return content; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Asynchrously get the result as stream from url. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I revised some of the |
||
| /// Send a GET request to the specified Uri with an HTTP completion option and a cancellation token as an asynchronous operation. | ||
| /// </summary> | ||
| /// <param name="url">The Uri the request is sent to.</param> | ||
| /// <param name="completionOption">An HTTP completion option value that indicates when the operation should be considered completed.</param> | ||
| /// <param name="token">A cancellation token that can be used by other objects or threads to receive notice of cancellation</param> | ||
| /// <returns></returns> | ||
| public static Task<Stream> GetStreamAsync([NotNull] string url, | ||
| CancellationToken token = default) => GetStreamAsync(new Uri(url), token); | ||
|
|
||
|
|
||
| /// <summary> | ||
| /// Send a GET request to the specified Uri with an HTTP completion option and a cancellation token as an asynchronous operation. | ||
| /// </summary> | ||
| /// <param name="url"></param> | ||
| /// <param name="token"></param> | ||
| /// <returns></returns> | ||
| public static async Task<Stream> GetStreamAsync([NotNull] string url, CancellationToken token = default) | ||
| public static async Task<Stream> GetStreamAsync([NotNull] Uri url, | ||
| CancellationToken token = default) | ||
| { | ||
| Log.Debug($"|Http.Get|Url <{url}>"); | ||
| return await client.GetStreamAsync(url, token); | ||
| } | ||
|
|
||
| public static async Task<HttpResponseMessage> GetResponseAsync(string url, HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead, | ||
| CancellationToken token = default) | ||
| => await GetResponseAsync(new Uri(url), completionOption, token); | ||
|
|
||
| public static async Task<HttpResponseMessage> GetResponseAsync([NotNull] Uri url, HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead, | ||
| CancellationToken token = default) | ||
| { | ||
| Log.Debug($"|Http.Get|Url <{url}>"); | ||
| var response = await client.GetAsync(url, HttpCompletionOption.ResponseHeadersRead, token); | ||
| return await response.Content.ReadAsStreamAsync(); | ||
| return await client.GetAsync(url, completionOption, token); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Asynchrously send an HTTP request. | ||
| /// </summary> | ||
| public static async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken token = default) | ||
| public static async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead, CancellationToken token = default) | ||
| { | ||
| return await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, token); | ||
| return await client.SendAsync(request, completionOption, token); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,11 +3,14 @@ | |
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.Net; | ||
| using System.Net.Http; | ||
| using System.Threading.Tasks; | ||
| using System.Windows.Media; | ||
| using System.Windows.Media.Imaging; | ||
| using Flow.Launcher.Infrastructure.Logger; | ||
| using Flow.Launcher.Infrastructure.Storage; | ||
| using static Flow.Launcher.Infrastructure.Http.Http; | ||
|
|
||
| namespace Flow.Launcher.Infrastructure.Image | ||
| { | ||
|
|
@@ -23,13 +26,7 @@ public static class ImageLoader | |
|
|
||
| private static readonly string[] ImageExtensions = | ||
| { | ||
| ".png", | ||
| ".jpg", | ||
| ".jpeg", | ||
| ".gif", | ||
| ".bmp", | ||
| ".tiff", | ||
| ".ico" | ||
| ".png", ".jpg", ".jpeg", ".gif", ".bmp", ".tiff", ".ico" | ||
| }; | ||
|
|
||
| public static void Initialize() | ||
|
|
@@ -39,21 +36,24 @@ public static void Initialize() | |
|
|
||
| var usage = LoadStorageToConcurrentDictionary(); | ||
|
|
||
| foreach (var icon in new[] { Constant.DefaultIcon, Constant.MissingImgIcon }) | ||
| foreach (var icon in new[] | ||
| { | ||
| Constant.DefaultIcon, Constant.MissingImgIcon | ||
| }) | ||
| { | ||
| ImageSource img = new BitmapImage(new Uri(icon)); | ||
| img.Freeze(); | ||
| ImageCache[icon] = img; | ||
| } | ||
|
|
||
| _ = Task.Run(() => | ||
| _ = Task.Run(async () => | ||
| { | ||
| Stopwatch.Normal("|ImageLoader.Initialize|Preload images cost", () => | ||
| await Stopwatch.NormalAsync("|ImageLoader.Initialize|Preload images cost", async () => | ||
| { | ||
| ImageCache.Data.AsParallel().ForAll(x => | ||
| foreach (var imageUsage in ImageCache.Data) | ||
| { | ||
| Load(x.Key); | ||
| }); | ||
| await LoadAsync(imageUsage.Key); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Due to the ImageLoader change to async (not always async so |
||
| } | ||
| }); | ||
| Log.Info($"|ImageLoader.Initialize|Number of preload images is <{ImageCache.CacheSize()}>, Images Number: {ImageCache.CacheSize()}, Unique Items {ImageCache.UniqueImagesInCache()}"); | ||
| }); | ||
|
|
@@ -99,7 +99,7 @@ private enum ImageType | |
| Cache | ||
| } | ||
|
|
||
| private static ImageResult LoadInternal(string path, bool loadFullImage = false) | ||
| private static async ValueTask<ImageResult> LoadInternalAsync(string path, bool loadFullImage = false) | ||
| { | ||
| ImageResult imageResult; | ||
|
|
||
|
|
@@ -113,7 +113,28 @@ private static ImageResult LoadInternal(string path, bool loadFullImage = false) | |
| { | ||
| return new ImageResult(ImageCache[path], ImageType.Cache); | ||
| } | ||
|
|
||
| if (Uri.TryCreate(path, UriKind.RelativeOrAbsolute, out var uriResult) | ||
| && (uriResult.Scheme == Uri.UriSchemeHttp || uriResult.Scheme == Uri.UriSchemeHttps)) | ||
| { | ||
| // Download image from url | ||
| await using var resp = await GetStreamAsync(uriResult); | ||
| if (resp == null) | ||
| { | ||
| return new ImageResult(ImageCache[Constant.MissingImgIcon], ImageType.Error); | ||
| } | ||
| await using var buffer = new MemoryStream(); | ||
| await resp.CopyToAsync(buffer); | ||
| buffer.Seek(0, SeekOrigin.Begin); | ||
| var image = new BitmapImage(); | ||
| image.BeginInit(); | ||
| image.CacheOption = BitmapCacheOption.OnLoad; | ||
| image.StreamSource = buffer; | ||
| image.EndInit(); | ||
| image.StreamSource = null; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to make the streamsource collectable |
||
| image.Freeze(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Freeze is needed because the BitmapImage is not created in the ui thread. |
||
| ImageCache[path] = image; | ||
| return new ImageResult(image, ImageType.ImageFile); | ||
| } | ||
| if (path.StartsWith("data:", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var imageSource = new BitmapImage(new Uri(path)); | ||
|
|
@@ -218,9 +239,9 @@ public static bool CacheContainImage(string path) | |
| return ImageCache.ContainsKey(path) && ImageCache[path] != null; | ||
| } | ||
|
|
||
| public static ImageSource Load(string path, bool loadFullImage = false) | ||
| public static async ValueTask<ImageSource> LoadAsync(string path, bool loadFullImage = false) | ||
| { | ||
| var imageResult = LoadInternal(path, loadFullImage); | ||
| var imageResult = await LoadInternalAsync(path, loadFullImage); | ||
|
|
||
| var img = imageResult.ImageSource; | ||
| if (imageResult.ImageType != ImageType.Error && imageResult.ImageType != ImageType.Cache) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -58,7 +58,13 @@ public string IcoPath | |||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||
| if (!string.IsNullOrEmpty(PluginDirectory) && !Path.IsPathRooted(value)) | ||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @taooceros & @Garulf not related to this PR but I am trying to figure out why we need to check PluginDirectory is not an empty string? I cant seem to hit this condition. |
||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||
| _icoPath = Path.Combine(value, IcoPath); | ||||||||||||||||||||||||||||||||||||||||||
| string absPath = Path.Combine(value, IcoPath); | ||||||||||||||||||||||||||||||||||||||||||
| // Only convert relative paths if its a valid path | ||||||||||||||||||||||||||||||||||||||||||
| if (File.Exists(absPath)) | ||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there another way to check this path without using File.Exists? Doesn't seem necessary to use it here.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This way allows us to improve how Flow auto appends the plugins path to icon paths that are relative and still accept URI such as http, https, ftp, etc. instead of checking every URI under the sun to see if it's valid let's just detect if that file even exists and leave it alone if it doesn't. Edit: original code just tacked on the plugins path to any string it didn't detect a root file path on.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean this is an unnecessary call because we already checking that the path exists or not at Flow.Launcher/Flow.Launcher.Infrastructure/Image/ImageLoader.cs Lines 183 to 202 in e5948a7
(GetThumbnailResult is called by LoadInternalAsync) We just need to determine that it is not a URI.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not really concerned with if the path exists or not. What we're really concerned with is the path really a relative path or not. The built-in method for detecting relative paths isnt very smart.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Agree, using Uri class is not working for our cases. Just doing a simple check for http and https seems more effective to determine that it is Url, then consider everything else as local absolute/relative path. @Garulf @taooceros I reworked IcoPath, PluginDirectory and ImageLoader 83ec809, let me know if ok. |
||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||
| _icoPath = Path.Combine(value, IcoPath); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -135,9 +141,15 @@ public string PluginDirectory | |||||||||||||||||||||||||||||||||||||||||
| set | ||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||
| _pluginDirectory = value; | ||||||||||||||||||||||||||||||||||||||||||
| if (!string.IsNullOrEmpty(IcoPath) && !Path.IsPathRooted(IcoPath)) | ||||||||||||||||||||||||||||||||||||||||||
| if (!string.IsNullOrEmpty(IcoPath) && !Path.IsPathRooted(IcoPath)) | ||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||
| IcoPath = Path.Combine(value, IcoPath); | ||||||||||||||||||||||||||||||||||||||||||
|
jjw24 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||
| string absPath = Path.Combine(value, IcoPath); | ||||||||||||||||||||||||||||||||||||||||||
| // Only convert relative paths if its a valid path | ||||||||||||||||||||||||||||||||||||||||||
| if (File.Exists(absPath)) | ||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||
| IcoPath = absPath; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.