Conversation
| Caution Review failedThe pull request is closed. WalkthroughAdds bidirectional Markdown import/export: CommonMark dependencies, Markdown parsing/rendering utilities, integration into import/export workflows, refactors export flows to file-based APIs with snackbar feedback, updates UI strings for Markdown support, and adds unit tests and resources. Changes
Sequence Diagram(s)sequenceDiagram participant User participant EditActivity participant BaseNoteModel participant ExportExtensions participant FileSystem participant UI User->>EditActivity: request export to file EditActivity->>BaseNoteModel: exportNoteToFile(fileUri, note, rootView) BaseNoteModel->>ExportExtensions: exportNote(note, mimeType, launcher) alt mime = MD ExportExtensions->>MarkdownUtils: createMarkdownFromBodyAndSpans(body, spans) else mime = PDF ExportExtensions->>ExportExtensions: exportPdfFile(...) else plaintext ExportExtensions->>ExportExtensions: exportPlainTextFile(...) end ExportExtensions->>FileSystem: write document ExportExtensions->>UI: showFileSnackbar(success, fileUri, mime) UI->>User: snackbar with action sequenceDiagram participant User participant PlainTextImporter participant MarkdownUtils participant Model User->>PlainTextImporter: import file alt file is Markdown PlainTextImporter->>MarkdownUtils: parseBodyAndSpansFromMarkdown(content) MarkdownUtils-->>PlainTextImporter: (body, spans) else list/plain text PlainTextImporter->>PlainTextImporter: parse as list/plain text end PlainTextImporter->>Model: create BaseNote(body/spans or list) Model-->>User: note created Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/translations.xlsxis excluded by!**/*.xlsx
📒 Files selected for processing (25)
README.md(1 hunks)app/build.gradle.kts(1 hunks)app/src/main/java/com/philkes/notallyx/data/imports/markdown/MarkdownUtils.kt(1 hunks)app/src/main/java/com/philkes/notallyx/data/imports/txt/PlainTextImporter.kt(4 hunks)app/src/main/java/com/philkes/notallyx/data/model/ModelExtensions.kt(2 hunks)app/src/main/java/com/philkes/notallyx/presentation/UiExtensions.kt(2 hunks)app/src/main/java/com/philkes/notallyx/presentation/activity/main/MainActivity.kt(2 hunks)app/src/main/java/com/philkes/notallyx/presentation/activity/note/EditActivity.kt(3 hunks)app/src/main/java/com/philkes/notallyx/presentation/viewmodel/BaseNoteModel.kt(11 hunks)app/src/main/java/com/philkes/notallyx/utils/AndroidExtensions.kt(1 hunks)app/src/main/java/com/philkes/notallyx/utils/backup/ExportExtensions.kt(8 hunks)app/src/main/res/values-cs/strings.xml(1 hunks)app/src/main/res/values-de/strings.xml(1 hunks)app/src/main/res/values-es/strings.xml(1 hunks)app/src/main/res/values-fr/strings.xml(1 hunks)app/src/main/res/values-it/strings.xml(1 hunks)app/src/main/res/values-pl/strings.xml(1 hunks)app/src/main/res/values-ro/strings.xml(1 hunks)app/src/main/res/values-ru/strings.xml(1 hunks)app/src/main/res/values-zh-rCN/strings.xml(1 hunks)app/src/main/res/values-zh-rTW/strings.xml(1 hunks)app/src/main/res/values/strings.xml(1 hunks)app/src/test/java/com/philkes/notallyx/data/imports/markdown/MarkdownUtilsTest.kt(1 hunks)app/src/test/java/com/philkes/notallyx/data/model/MarkdownExportTest.kt(1 hunks)app/src/test/resources/markdown/all_spans_and_image.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
app/src/main/java/com/philkes/notallyx/data/imports/txt/PlainTextImporter.kt (1)
app/src/main/java/com/philkes/notallyx/data/imports/markdown/MarkdownUtils.kt (1)
parseBodyAndSpansFromMarkdown(16-131)
app/src/test/java/com/philkes/notallyx/data/imports/markdown/MarkdownUtilsTest.kt (2)
app/src/test/java/com/philkes/notallyx/data/model/MarkdownExportTest.kt (2)
note(6-125)note(8-29)app/src/main/java/com/philkes/notallyx/data/imports/markdown/MarkdownUtils.kt (1)
parseBodyAndSpansFromMarkdown(16-131)
app/src/main/java/com/philkes/notallyx/presentation/activity/note/EditActivity.kt (1)
app/src/main/java/com/philkes/notallyx/utils/backup/ExportExtensions.kt (1)
exportNote(828-846)
app/src/main/java/com/philkes/notallyx/data/model/ModelExtensions.kt (1)
app/src/main/java/com/philkes/notallyx/data/imports/markdown/MarkdownUtils.kt (1)
createMarkdownFromBodyAndSpans(138-193)
app/src/main/java/com/philkes/notallyx/presentation/viewmodel/BaseNoteModel.kt (2)
app/src/main/java/com/philkes/notallyx/utils/backup/ExportExtensions.kt (4)
exportPdfFile(560-600)exportPlainTextFile(642-668)exportPdfFileFolder(523-558)exportPlainTextFileFolder(602-640)app/src/main/java/com/philkes/notallyx/presentation/UiExtensions.kt (2)
showSnackbar(931-935)showSnackbar(937-944)
app/src/test/java/com/philkes/notallyx/data/model/MarkdownExportTest.kt (1)
app/src/test/java/com/philkes/notallyx/data/imports/markdown/MarkdownUtilsTest.kt (2)
note(9-110)note(11-32)
🔇 Additional comments (22)
app/src/main/java/com/philkes/notallyx/utils/AndroidExtensions.kt (1)
375-386: Remove the redundant FLAG_ACTIVITY_NEW_TASK from the base intent.Adding
FLAG_ACTIVITY_NEW_TASKto both the base intent and the chooser intent is non-standard. The codebase already has a similar file-viewing function (around line 407-419) that works without either flag. The standard Android pattern is:
- Grant flags (like
FLAG_GRANT_READ_URI_PERMISSION) on the base intent- Launch flags (like
FLAG_ACTIVITY_NEW_TASK) only on the outer intent being startedRemove line 380 (
addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)on the base intent) and keep only the flag on the chooser (line 384), matching the pattern used elsewhere in the codebase.app/src/main/res/values-cs/strings.xml (1)
252-253: Localization change appropriately reflects Markdown support.The label prefix aligns with the help text mentioning Markdown syntax detection, and matches updates across other localized strings.
app/src/main/res/values-zh-rTW/strings.xml (1)
235-235: Consistent localization update across supported languages.This change mirrors updates to other locale files and aligns with Markdown support messaging in help text.
app/src/main/res/values-es/strings.xml (1)
251-252: Localization correctly reflects Markdown support in Spanish.Translation maintains consistency with other language variants and aligns with updated help messaging.
app/src/main/res/values-zh-rCN/strings.xml (1)
244-244: Localization update aligns with Markdown support across all locales.Simplified Chinese translation maintains consistency and clarity.
app/src/main/res/values-it/strings.xml (1)
230-231: Italian localization properly updated to reflect Markdown support.Translation is idiomatic and consistent with other language versions.
app/src/main/res/values-de/strings.xml (1)
251-252: German localization correctly indicates Markdown support.Translation maintains consistency with translations in other languages.
app/src/test/resources/markdown/all_spans_and_image.md (1)
1-5: Test resource appropriately covers Markdown formatting scenarios.The file tests a comprehensive range of inline spans (bold, italic, code, strikethrough, links, images) and nested formatting, which aligns well with the Markdown parsing/rendering utility test cases mentioned in the PR context.
app/build.gradle.kts (1)
270-271: Dependencies verified as appropriate and secure.Version 0.27.0 is current on javadoc.io and available on Maven Central, with no published security advisories for commonmark-java. GFM strikethrough extension is fully supported in this version. The dependencies are suitable for the Markdown parsing use case.
app/src/main/res/values-ro/strings.xml (1)
238-238: LGTM! Localization update aligns with Markdown support.The Romanian translation correctly reflects the new Markdown/plain text file support introduced in this PR.
app/src/main/res/values-pl/strings.xml (1)
249-249: LGTM! Polish localization correctly updated.The string update properly reflects the Markdown/plain text labeling consistent with the PR's objectives.
app/src/main/res/values-ru/strings.xml (1)
252-252: LGTM! Russian localization updated consistently.The translation correctly includes the Markdown prefix consistent with other locale updates in this PR.
app/src/main/res/values-fr/strings.xml (1)
238-238: LGTM! French localization updated consistently.The translation appropriately reflects the Markdown/plain text file support.
app/src/main/res/values/strings.xml (1)
253-254: LGTM! Base string resources updated consistently.Both the label and help text correctly reflect the new Markdown support introduced in this PR.
README.md (1)
93-94: Minor formatting change - two trailing blank lines added.This appears to be an incidental formatting change, possibly from editor auto-formatting. No functional impact.
app/src/main/java/com/philkes/notallyx/presentation/activity/note/EditActivity.kt (3)
91-91: LGTM! New import for refactored export flow.The import of the
exportNoteextension function supports the refactored single-note export workflow.
405-412: LGTM! Result handler correctly updated for single-note export.The activity result handler now directly calls
baseModel.exportNoteToFilewith the selected URI, note, and root view for snackbar feedback. This aligns with the refactored export API.
958-960: LGTM! Export method correctly uses new extension function.The
exportmethod now invokes theexportNoteextension function which handles note selection and file picker launch. This is consistent with the refactored export flow.app/src/main/java/com/philkes/notallyx/data/imports/txt/PlainTextImporter.kt (4)
9-9: LGTM! New imports support Markdown functionality.The imports for Markdown parsing, MIME type handling, and logging are all properly utilized in the updated import logic.
Also applies to: 15-15, 17-17
55-70: LGTM! Well-structured Markdown parsing with proper error handling.The code appropriately:
- Only attempts Markdown parsing when the file is identified as Markdown and no list items are detected
- Includes exception handling with detailed logging for debugging
- Falls back to plain text content if parsing fails
- Returns an empty spans list for non-Markdown content
This defensive approach ensures robust handling of edge cases.
83-84: LGTM! Conditional body/spans assignment is correct.The logic appropriately clears body and spans for list notes (when
listItems.isEmpty()is false) and uses the parsed Markdown body/spans for text notes.
108-117: LGTM! Markdown file detection is correctly implemented.The
isMarkdownFile()helper properly identifies Markdown files by checking both:
- MIME type matching (case-insensitive)
- File extension matching (case-insensitive)
The companion object with TAG constant follows standard Android logging practices.
app/src/main/java/com/philkes/notallyx/data/imports/txt/PlainTextImporter.kt Show resolved Hide resolved
app/src/main/java/com/philkes/notallyx/presentation/viewmodel/BaseNoteModel.kt Outdated Show resolved Hide resolved
| fun exportSelectedNoteToFile(fileUri: Uri, snackbarView: View) { | ||
| exportNoteToFile(fileUri, actionMode.selectedNotes.values.first(), snackbarView) | ||
| } |
There was a problem hiding this comment.
Fix single-note export path to avoid crash
After launching the document picker we store the note in selectedExportBaseNote, but here we still pull from actionMode.selectedNotes. If the export was triggered outside of multi-select (or the selection was cleared/config-changed before the result returns), this throws NoSuchElementException and the export fails. Use the saved note as the primary source and clear it once consumed.
Apply this diff to use the stored note safely:
fun exportSelectedNoteToFile(fileUri: Uri, snackbarView: View) { - exportNoteToFile(fileUri, actionMode.selectedNotes.values.first(), snackbarView) + val note = + selectedExportBaseNote ?: actionMode.selectedNotes.values.firstOrNull() + ?: throw IllegalStateException("No note selected for export") + selectedExportBaseNote = null + exportNoteToFile(fileUri, note, snackbarView) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun exportSelectedNoteToFile(fileUri: Uri, snackbarView: View) { | |
| exportNoteToFile(fileUri, actionMode.selectedNotes.values.first(), snackbarView) | |
| } | |
| fun exportSelectedNoteToFile(fileUri: Uri, snackbarView: View) { | |
| val note = | |
| selectedExportBaseNote ?: actionMode.selectedNotes.values.firstOrNull() | |
| ?: throw IllegalStateException("No note selected for export") | |
| selectedExportBaseNote = null | |
| exportNoteToFile(fileUri, note, snackbarView) | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/philkes/notallyx/presentation/viewmodel/BaseNoteModel.kt around lines 574-576, the exportSelectedNoteToFile currently pulls the note from actionMode.selectedNotes which can throw NoSuchElementException if selection was cleared; instead use the stored selectedExportBaseNote as the primary source (fallback to actionMode.selectedNotes only if that is null), pass that note into exportNoteToFile, and clear selectedExportBaseNote after consuming it so subsequent operations don't reuse stale state. 
Crustack force-pushed the feature/markdown-export-import branch from 83ecab4 to 4c8e78b Compare 
1 participant
Closes #713
Summary by CodeRabbit
New Features
Localization
Tests