Add clean cache option & Add exception handler for cleaning folders #3411
Conversation
Jack251970
commented
Apr 1, 2025
Jack251970
commented
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new clean cache option to the settings view, updating translation calls across the board and introducing functionality for clearing the cache folder.
- Updated translation calls to use App.API.GetTranslation
- Added a new CacheFolderSize property to display the cache size
- Introduced AskClearCacheFolderConfirmation and supporting methods to clear the cache folder
Files not reviewed (2)
- Flow.Launcher/Languages/en.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneAbout.xaml: Language not supported
Comments suppressed due to low confidence (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs:173
- Consider checking if the cache directory exists (e.g. using cacheDirectory.Exists) before enumerating files and directories to avoid potential runtime exceptions when the directory is missing.
private void ClearCacheFolder()
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: Yusyuriv, onesounds Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
📝 WalkthroughWalkthroughThis pull request introduces enhancements to cache management in the application. Three new string resources related to cache clearing messages are added to the language file. The settings view model is updated with a new property for cache size and methods for prompting user confirmation and clearing the cache. Additionally, a new button is added in the view to trigger the cache clearing functionality, along with updates to localization calls and method names for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant VM as ViewModel
participant FS as FileSystem
User->>UI: Click "Clear Caches" button
UI->>VM: Execute AskClearCacheFolderConfirmationCommand
VM->>User: Show confirmation dialog ("Are you sure you want to delete all caches?")
User-->>VM: Confirm
VM->>FS: Delete cache files
FS-->>VM: Deletion complete
VM->>UI: Update CacheFolderSize property
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs (1)
149-161: Refactor duplicated code in folder clearing methods.The
ClearLogFolderandClearCacheFoldermethods contain very similar code with only minor differences, which introduces maintenance challenges.Consider refactoring to a more generic method:
+ private void ClearFolder(DirectoryInfo directory, Func<List<FileInfo>> getFiles, string propertyToUpdate, bool deleteSubdirectories = false, Func<DirectoryInfo, bool> directoryFilter = null) + { + var files = getFiles(); + + try { + files.ForEach(f => { + try { + f.Delete(); + } catch (Exception ex) when (ex is IOException || ex is UnauthorizedAccessException) { + Log.Error($"Failed to delete file {f.FullName}: {ex.Message}", ex); + } + }); + + var directories = directory.EnumerateDirectories("*", SearchOption.TopDirectoryOnly); + if (directoryFilter != null) + directories = directories.Where(directoryFilter); + + directories.ToList().ForEach(dir => { + try { + dir.Delete(deleteSubdirectories); + } catch (Exception ex) when (ex is IOException || ex is UnauthorizedAccessException) { + Log.Error($"Failed to delete directory {dir.FullName}: {ex.Message}", ex); + } + }); + } finally { + OnPropertyChanged(propertyToUpdate); + } + } + - private void ClearLogFolder() - { - var logDirectory = GetLogDir(); - var logFiles = GetLogFiles(); - - logFiles.ForEach(f => f.Delete()); - - logDirectory.EnumerateDirectories("*", SearchOption.TopDirectoryOnly) - .Where(dir => !Constant.Version.Equals(dir.Name)) - .ToList() - .ForEach(dir => dir.Delete()); - - OnPropertyChanged(nameof(LogFolderSize)); - } + private void ClearLogFolder() + { + ClearFolder( + GetLogDir(), + GetLogFiles, + nameof(LogFolderSize), + false, + dir => !Constant.Version.Equals(dir.Name)); + } - private void ClearCacheFolder() - { - var cacheDirectory = GetCacheDir(); - var cacheFiles = GetCacheFiles(); - - cacheFiles.ForEach(f => f.Delete()); - - cacheDirectory.EnumerateDirectories("*", SearchOption.TopDirectoryOnly) - .ToList() - .ForEach(dir => dir.Delete(true)); - - OnPropertyChanged(nameof(CacheFolderSize)); - } + private void ClearCacheFolder() + { + ClearFolder( + GetCacheDir(), + GetCacheFiles, + nameof(CacheFolderSize), + true); + }This refactoring reduces code duplication, adds error handling, and makes future maintenance easier.
Also applies to: 173-185
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher/Languages/en.xaml(1 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs(6 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneAbout.xaml(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs (4)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (4)
RelayCommand(241-251)RelayCommand(253-263)RelayCommand(265-270)RelayCommand(272-277)Flow.Launcher/ViewModel/PluginViewModel.cs (7)
RelayCommand(153-158)RelayCommand(160-166)RelayCommand(168-172)RelayCommand(174-179)RelayCommand(181-186)RelayCommand(188-193)Task(46-50)Flow.Launcher/ViewModel/MainViewModel.cs (10)
RelayCommand(272-280)RelayCommand(282-294)RelayCommand(296-304)RelayCommand(313-324)RelayCommand(326-337)RelayCommand(339-353)Task(259-270)Task(641-668)Task(900-927)Task(951-955)Flow.Launcher.Core/Plugin/PluginManager.cs (11)
Task(78-89)Task(91-99)Task(101-108)Task(110-117)Task(119-126)Task(187-244)Task(260-302)Task(467-472)Task(485-488)Task(568-635)List(335-362)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (8)
Flow.Launcher/Languages/en.xaml (1)
328-329: New string resources added for cache cleaning feature.The new string resources follow the same pattern as existing log clearing resources, providing clear and consistent messaging for the new cache clearing functionality.
Flow.Launcher/SettingPages/Views/SettingsPaneAbout.xaml (1)
93-96: New button for clearing cache added.The button is properly positioned alongside the existing log clearing button, with consistent margin and binding pattern. It uses the CacheFolderSize property to display the current size of the cache.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs (6)
22-28: Translation API method updated.Changed from using
InternationalizationManager.Instance.GetTranslationtoApp.API.GetTranslationfor consistent API usage throughout the application.
30-36: New CacheFolderSize property added.The property calculates the total size of all cache files using the same pattern as LogFolderSize, providing a consistent user experience.
53-53: Translation API method updated.Consistent update from
InternationalizationManager.Instance.GetTranslationtoApp.API.GetTranslation.
99-100: Translation API method updated.Consistent update from
InternationalizationManager.Instance.GetTranslationtoApp.API.GetTranslation.
110-123: New method added for confirming cache folder clearing.The method follows the same pattern as the existing log folder confirmation, providing a consistent user experience.
146-146: Method signature changed to indicate async operation.The method signature has been updated from
UpdateApptoUpdateAppAsyncto better reflect its asynchronous nature.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Jack251970
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds a clean cache option and introduces exception handling for cleaning folders, while also updating translation calls from InternationalizationManager to App.API.
- Replaces translation calls for internationalized strings
- Adds a new CacheFolderSize property and methods for clearing cache and retrieving cache files
- Introduces exception handling in folder cleaning methods and renames a method for clarity
Files not reviewed (2)
- Flow.Launcher/Languages/en.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneAbout.xaml: Language not supported
Comments suppressed due to low confidence (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs:162
- The method has been renamed to 'UpdateAppAsync' to reflect its asynchronous behavior; please ensure that all command bindings and invocations are updated accordingly.
private Task UpdateAppAsync() => _updater.UpdateAppAsync(false);
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a clean cache option and robust error handling for the cleaning of log and cache folders. Key changes include replacing translations from InternationalizationManager with App.API, adding a new property and confirmation command for cleaning the cache folder, and incorporating try-catch error handling for file and directory deletions.
Files not reviewed (2)
- Flow.Launcher/Languages/en.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneAbout.xaml: Language not supported
Comments suppressed due to low confidence (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs:183
- In ClearLogFolder the deletion of directories uses a non-recursive Delete() method, while ClearCacheFolder uses dir.Delete(true) for recursive deletion. Please verify that this difference is intentional to ensure consistent folder cleaning behavior.
dir.Delete();
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an option to clean the cache and enhances error handling during folder clean-up operations. Key changes include:
- Removing the unused resource import and updating translation calls to use App.API.
- Adding a new CacheFolderSize property along with a ClearCacheFolder() method to handle cache cleaning with exception handling.
- Renaming the asynchronous update method to UpdateAppAsync and incorporating improved error handling in log folder deletion.
Files not reviewed (2)
- Flow.Launcher/Languages/en.xaml: Language not supported
- Flow.Launcher/SettingPages/Views/SettingsPaneAbout.xaml: Language not supported
Comments suppressed due to low confidence (3)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs:154
- Ensure that all callers and bindings referring to the previous UpdateApp method are updated to use the new UpdateAppAsync name for consistency with asynchronous conventions.
private Task UpdateAppAsync() => _updater.UpdateAppAsync(false);
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs:170
- [nitpick] Consider catching more specific exceptions rather than a generic Exception in the log file deletion block to handle expected file system errors more accurately.
App.API.LogException(ClassName, "Failed to delete log file: {f.Name}", e);
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs:236
- [nitpick] Consider catching more specific exceptions rather than a generic Exception in the cache directory deletion block to better distinguish and manage different error conditions.
App.API.LogException(ClassName, "Failed to delete cache directory: {dir.Name}", e);
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
