Restore Document category and applications - #4902
Conversation
Moves existing document-focused apps into the Document category and adds its filter chip.
Restores reviewed document-focused apps removed in PR ChrisTitusTech#4451.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe application catalog reclassifies document-related applications and adds new records. The Install tab now includes a Document filter chip, its handler applies the Document category, and XAML tests cover the new control. ChangesDocument category
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Installer as Install tab
participant Handler as Document chip handler
participant Filter as Set-WinUtilAppCategoryFilter
participant Catalog as applications.json
Installer->>Handler: Click WPFSearchChipDocument
Handler->>Filter: Apply Category "Document"
Filter->>Catalog: Select Document applications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
new feature
![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)
pester/xaml.Tests.ps1 (1)
145-145: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover the new cross-file filter contract.
This assertion only verifies that the XAML control exists. Add assertions that
scripts/main.ps1bindsWPFSearchChipDocumentto-Category "Document"and that the intended records inconfig/applications.jsonuseCategory = "Document".The existing exact-category behavior test in
pester/search-filter.Tests.ps1does not cover this new chip wiring.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pester/xaml.Tests.ps1` at line 145, Extend the tests around the WPFSearchChipDocument assertion in xaml.Tests.ps1 to verify that scripts/main.ps1 binds the chip to -Category "Document" and that the intended records in config/applications.json declare Category = "Document". Keep the existing control-existence assertion and exact-category test unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/applications.json`:
- Around line 1117-1133: Update the content values for the pdf-xchange and
pdf24creator application entries to use the canonical display names “PDF-XChange
Editor” and “PDF24 Creator” respectively; leave the remaining metadata
unchanged.
---
Nitpick comments:
In `@pester/xaml.Tests.ps1`:
- Line 145: Extend the tests around the WPFSearchChipDocument assertion in
xaml.Tests.ps1 to verify that scripts/main.ps1 binds the chip to -Category
"Document" and that the intended records in config/applications.json declare
Category = "Document". Keep the existing control-existence assertion and
exact-category test unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8863dc30-1996-411b-9786-1fc8322dd59d
📒 Files selected for processing (4)
config/applications.jsonpester/xaml.Tests.ps1scripts/main.ps1xaml/inputXML.xaml
This also updates the link for PDF24 Creator
Covers the Document filter handler and config category presence.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pester/xaml.Tests.ps1 (1)
145-145: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the chip label as well as the control name.
The current assertion only checks
WPFSearchChipDocument. A control withContent="Other"would pass while the user-visible filter label is incorrect. Add an assertion forContent="Document".Proposed assertion
+ $documentChip = $script:xaml.SelectSingleNode('//*[local-name()="Button"][`@Name`="WPFSearchChipDocument"]') + $documentChip.GetAttribute("Content") | Should -Be "Document"Based on the PR objective, the chip must expose the
Documentlabel.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pester/xaml.Tests.ps1` at line 145, Update the test assertion for WPFSearchChipDocument to also verify its user-visible Content is exactly “Document,” while preserving the existing control-name assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pester/xaml.Tests.ps1`:
- Line 145: Update the test assertion for WPFSearchChipDocument to also verify
its user-visible Content is exactly “Document,” while preserving the existing
control-name assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 432e52fd-cc53-4d0c-93bf-2f08ed0523e5
📒 Files selected for processing (1)
pester/xaml.Tests.ps1
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01617bb3d9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
The reasons that PR happened were both having too much apps and most apps being too niche. Also 4451 was reviewed live on stream and discussed with audience, so all the decisions and agreements from Chris himself were told and discussed. The only thing I could agree is PDF24 as PDF toolkit, but most of other things were removed for a reason. |
I understand that #4451 was discussed and reviewed at the time, but I don't think the fact that an application was removed in that PR is, by itself, sufficient justification for keeping it removed. Some applications removed by #4451 have already been restored since then, so those decisions clearly aren't immutable. That's why this PR evaluates the applications individually. Joplin is a good example: it's one of Obsidian's biggest competitors. Calling something like Joplin "too niche" without a specific reason doesn't seem convincing to me. Also, having too many apps isn't a justification to remove an entire category. #4889 loads them without blocking the UI and faster, and the filters/search are useful to find things quickly. The same applies to the category itself. There were already six document-focused applications in the current catalog sitting under Multimedia Tools, and this PR brings the category to 17 applications with clear document-related use cases. I think that's more than enough to justify a dedicated Document category. If there are specific applications here that you think shouldn't be included, I'm completely open to discussing them individually and removing them if the reasoning makes sense. But I don't think "they were removed in #4451" or "the PR was reviewed on stream" addresses the individual justifications provided here unless Chris has a different opinion now. |
|
Okular is known for Linux DE's built-in viewer, most people still choose anything else at Windows, because there is something else that will be better than this. PDF24 actually is web toolkit, so I will call my words back about it, makes no sense to install a tool which is web-based, unless you can't have Internet access most of time. (unless that's an app that depends on system resources, which can require some special libraries etc, etc, etc) All the others are unheard by majority of people or considered niche comparing to bigger and better alternatives, which why they were removed |
Thanks for giving some app-specific examples. This is the kind of discussion I was asking for. On Okular, I think that's a fair point. Its Windows audience is much smaller and it's primarily associated with KDE/Linux, so I'm okay removing Okular from this PR. For Joplin, though, "can't deny, can't agree" isn't really a justification. I'm also not sure what "its appearance hasn't succeeded that much" is based on. Joplin is FOSS, cross-platform, supports E2EE, gives users control over where their data is synchronized, and can be used with self-hosted infrastructure. It's also recommended by major privacy/security resources: Obsidian and Notion being more popular doesn't make Joplin redundant. They overlap because they're note-taking applications, but they have different licensing models, workflows, sync options, and approaches to data ownership. 
The same logic applies to the rest of the catalog. Chrome and Firefox exist, but we still include alternatives like Brave, ungoogled chromium and Helium because people want different tradeoffs. Helium and ungoogled are probably "unheard of by the majority of people" compared to Chrome, Edge, Firefox, or Brave, but that doesn't make it a bad option to have. I also don't agree that a browser replaces dedicated PDF software. If that were enough justification, then Adobe Acrobat Reader shouldn't be in WinUtil either because browsers already open PDFs. Dedicated PDF readers and editors still have a market because people actually use and prefer them, and they provide different workflows and functionality. I personally prefer dedicated PDF software too. The restored PDF-related apps also aren't just copies of each other:
I'm also confused about PDF24 Creator. In your previous comment you said it was the one restoration you agreed with, but now you're arguing against it because PDF24 has web tools. PDF24 Creator is an actual Windows desktop application, separate from their web tools, and is specifically designed to provide that functionality locally/offline: https://tools.pdf24.org/en/creator I also think we really need to stop using "niche" by itself as justification. "Niche", "unheard of by the majority of people", or "there are bigger alternatives" don't actually explain why an application shouldn't be in the catalog. That same argument could be applied to Helium and plenty of applications already in Development, Selfhosted Tools, Pro Tools, Utilities, and other categories. I'm not arguing that every application belongs in WinUtil. Okular is a good example where you gave me a concrete reason to reconsider it, and I'm fine removing it. If an application has poor Windows support, is abandoned, has packaging problems, has questionable behavior, or is genuinely too specialized for WinUtil, give me that specific reason and I'm happy to make changes where it makes sense. But simply saying something is "niche" or that a more popular alternative exists isn't enough on its own. The point of the catalog is to provide a curated set of useful options, not one winner for every use case. If we fundamentally disagree on how broad that catalog should be, that's ultimately a product-direction decision and I'm fine leaving that to @ChrisTitusTech. Thanks. |
ChrisTitusTech
left a comment
ChrisTitusTech
left a comment
There was a problem hiding this comment.
Approved in CodeRabbit Change Stack
Keeps the e-book reader in Multimedia Tools.
3 participants
Type of Change
Description
This PR restores the
Documentapplication category removed in PR #4451 and adds its Install-tab filter chip.The restored applications were individually audited before this change. Each has an app-specific justification for inclusion in the WinUtil catalog; this is not a blanket reversal of PR #4451. The five existing applications moved into
Documentwere also reviewed for category fit. Please let me know if any application doesn't belong in the catalog, and if it makes sense I'm more than happy to make the change.Moved into the Document category
These applications already existed in WinUtil and were moved from
Multimedia Tools:Restored from PR #4451
Validation
./Compile.ps1Invoke-Pester -Path 'pester/*.Tests.ps1' -Output Normal- 471 passedgit diff --checkScreenshot
Issue related to PR