-
Notifications
You must be signed in to change notification settings - Fork 756
Added Documents viewer plugin id to load pdf/excel/csv etc. files #1833
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
Conversation
the Documents Viewer plugin/extension provides the view support for multiple types of documents like pdf, csv, xlsx, docs etc developed and maintained by user 1446 ( HACKESOFICE ). ## Problem > when user opens these files directly through file manager => open with Acode then Acode opens these files directly to the text editor, which is not useful as these files are binary files.## Reason> Acode at first ( startup ) loads only plugins which are listed in List / Arrary ( HEME_IDENTIFIERS ) then other plugins ( Documents viewer also ) are loaded after Successful app startup. so the file handlers are regestering after file load and user see's only bytes as text. ## Solution > Added The Documents Viewer plugin id ( Which is "documentsviewer" ) in HEME_IDENTIFIERS LIst / Array. ## Expected Workflow / behaviorUser > Clicks on Acode app => load all plugins with the id avilable in HEME_IDENTIFIERS ( Now Documents Viewer Also ) => then Load Files. ## Why It'll Work > Because of Acode will now load the plugin ( if avilable ) at startup so all the file handlers will be registered before loading/opening the particular files.
Summary of ChangesHello @hackesofice, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical user experience issue where common document types like PDFs, CSVs, and Excel files were being opened as raw text within the application's text editor. The core change involves adjusting the plugin loading sequence to ensure the dedicated 'Documents Viewer' plugin is initialized at startup. This guarantees that the appropriate file handlers are active when these document types are accessed, enabling them to be viewed correctly rather than as unreadable binary data. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Queued: Greptile will review this PR after it finishes indexing your repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request aims to load the 'documentsviewer' plugin at startup by adding it to the THEME_IDENTIFIERS set. While this achieves the desired functional outcome, it introduces a code clarity and maintainability issue by placing a non-theme plugin identifier into a list explicitly named for themes. My review provides feedback on how to address this to make the code's intent clearer for future maintenance.
| "moonlight", | ||
| "bluloco", | ||
| "acode.plugin.extra_syntax_highlights", | ||
| "documentsviewer", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding documentsviewer to THEME_IDENTIFIERS is semantically incorrect as this plugin is not a theme. This makes the code confusing and harder to maintain. The goal is to load this plugin at startup, and it seems THEME_IDENTIFIERS is being used for this purpose for other non-theme plugins as well (e.g., acode.plugin.extra_syntax_highlights), which points to a naming issue with the constant itself.
To improve code clarity, THEME_IDENTIFIERS should be renamed to something more generic, like STARTUP_PLUGIN_IDENTIFIERS. Consequently, the isThemePlugin function and its usages should also be renamed to reflect this broader purpose (e.g., isStartupPlugin). This will make the code's intent clear and improve long-term maintainability.
the Documents Viewer plugin/extension provides the view support for multiple types of documents like
pdf, csv, xlsx, docs etc..developed and maintained by user 1446HACKESOFICE.Problem
Reason
Expected Workflow / behavior
Why It'll Work