-
Notifications
You must be signed in to change notification settings - Fork 418
Add email and obsidian export #1148
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
📝 WalkthroughWalkthroughThis update introduces a new Obsidian integration across the desktop application and plugin ecosystem. It adds a new plugin ( Changes
Sequence Diagram(s)Obsidian Integration: Settings FlowsequenceDiagram
participant User
participant ReactUI as IntegrationsComponent
participant ObsidianPlugin as obsidianCommands (JS)
participant TauriBackend as Tauri Plugin (Rust)
User->>ReactUI: Open Settings > Integrations tab
ReactUI->>ObsidianPlugin: getEnabled(), getApiKey(), getBaseUrl()
ObsidianPlugin->>TauriBackend: get_enabled, get_api_key, get_base_url
TauriBackend-->>ObsidianPlugin: enabled, apiKey, baseUrl
ObsidianPlugin-->>ReactUI: enabled, apiKey, baseUrl
User->>ReactUI: Toggle enable, edit API key/base URL
ReactUI->>ObsidianPlugin: setEnabled(), setApiKey(), setBaseUrl()
ObsidianPlugin->>TauriBackend: set_enabled, set_api_key, set_base_url
TauriBackend-->>ObsidianPlugin: Result
ObsidianPlugin-->>ReactUI: Result
ReactUI->>ObsidianPlugin: getEnabled(), getApiKey(), getBaseUrl() (refetch)
ObsidianPlugin->>TauriBackend: get_enabled, get_api_key, get_base_url
TauriBackend-->>ObsidianPlugin: enabled, apiKey, baseUrl
ObsidianPlugin-->>ReactUI: enabled, apiKey, baseUrl
Share Button: Multi-Export FlowsequenceDiagram
participant User
participant ShareButton
participant ExportMutation
participant TauriBackend
User->>ShareButton: Click Share
ShareButton->>User: Show export options (PDF, Email, Obsidian)
User->>ShareButton: Select export option
ShareButton->>ExportMutation: exportMutation(option)
alt PDF
ExportMutation->>TauriBackend: export as PDF
TauriBackend-->>ExportMutation: PDF file
else Email
ExportMutation->>TauriBackend: open mailto URL
TauriBackend-->>ExportMutation: (handled)
else Obsidian
ExportMutation->>TauriBackend: export note to Obsidian plugin
TauriBackend-->>ExportMutation: deep link URL
ExportMutation-->>User: Open deep link URL
end
ExportMutation-->>ShareButton: Result
ShareButton-->>User: Show status/feedback
Possibly related PRs
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
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.
Actionable comments posted: 7
🧹 Nitpick comments (11)
plugins/obsidian/.gitignore (4)
1-1: Clarify Visual Studio folder ignore pattern
/.vsmatches either a file named.vsor a directory; appending a slash makes the intention explicit and prevents accidental false-positives.-/.vs +/.vs/
6-9: Re-consider ignoring JavaScript lock files
Excludingpackage-lock.jsonandyarn.lockremoves deterministic dependency resolution and can make builds harder to reproduce or debug. Unless there is a deliberate workflow that regenerates these locks in CI, keep the lock file committed.
13-13:Cargo.lockshould generally be committed for binary crates
Rust best practice is to commitCargo.lockfor applications (binaries) to guarantee repeatable builds. If this plugin builds an executable (Tauri backend), you may want to removeCargo.lockfrom the ignore list.
18-18: Add a terminating newline
Conventional POSIX tooling expects text files to end with a newline; adding one avoids needless diff noise later.-dist +dist +apps/desktop/src-tauri/capabilities/default.json (1)
54-55: Double-check wildcard semantics formailto:pattern
"mailto:*"might only match addresses without query params or multiple recipients. Tauri’s glob syntax typically mirrors**for “anything, including path separators”, so consider:- "allow": [{ "url": "https://**" }, { "url": "mailto:*" }] + "allow": [{ "url": "https://**" }, { "url": "mailto:**" }]to safely cover
mailto:user@example.com?subject=…and similar variants.
Please verify against the capability validator to avoid unexpected rejects at runtime.plugins/obsidian/permissions/default.toml (1)
1-3: Optional: add explicit$schemakey for editor / CI validationOther permission files include a schema reference; mirroring it here improves linting & IDE support:
+[ "$schema" ] +value = "./schemas/schema.json"Not strictly required at runtime, but helpful for tooling consistency.
plugins/obsidian/tsconfig.json (1)
1-5: Broadenincludeglob to future-proof against sub-foldersIf any JS helpers end up nested (e.g.,
js/utils/foo.ts) they’ll be skipped. A more flexible pattern:- "include": ["./js/*.ts"], + "include": ["./js/**/*.ts"],Same behaviour today, safer tomorrow.
plugins/obsidian/Cargo.toml (1)
8-8: Consider adding a description.The empty description field could be populated to better document the plugin's purpose.
-description = "" +description = "Tauri plugin for Obsidian integration"plugins/obsidian/src/ext.rs (1)
5-5: Simplify ping method return type.The
pingmethod always returnsOk(true)and never errors. Consider simplifying the return type to justbool.-fn ping(&self) -> Result<bool, String>; +fn ping(&self) -> bool; -fn ping(&self) -> Result<bool, String> { - Ok(true) -} +fn ping(&self) -> bool { + true +}Also applies to: 13-15
plugins/obsidian/permissions/schemas/schema.json (1)
133-133: Fix typo in description."scoped" should be "scopes".
- "description": "Allowed or denied scoped when using this permission.", + "description": "Allowed or denied scopes when using this permission.",apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)
170-174: Simplify the chevron button structure.The fragment and conditional wrapper are unnecessary since the button is always rendered.
- { - <button className="text-gray-500"> - {expanded ? <ChevronUp size={16} /> : <ChevronDown size={16} />} - </button> - } + <button className="text-gray-500"> + {expanded ? <ChevronUp size={16} /> : <ChevronDown size={16} />} + </button>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/desktop/src-tauri/capabilities/default.json(1 hunks)apps/desktop/src/components/toolbar/buttons/share-button.tsx(4 hunks)plugins/obsidian/.gitignore(1 hunks)plugins/obsidian/Cargo.toml(1 hunks)plugins/obsidian/build.rs(1 hunks)plugins/obsidian/js/bindings.gen.ts(1 hunks)plugins/obsidian/js/index.ts(1 hunks)plugins/obsidian/package.json(1 hunks)plugins/obsidian/permissions/autogenerated/commands/ping.toml(1 hunks)plugins/obsidian/permissions/autogenerated/reference.md(1 hunks)plugins/obsidian/permissions/default.toml(1 hunks)plugins/obsidian/permissions/schemas/schema.json(1 hunks)plugins/obsidian/src/commands.rs(1 hunks)plugins/obsidian/src/error.rs(1 hunks)plugins/obsidian/src/ext.rs(1 hunks)plugins/obsidian/src/lib.rs(1 hunks)plugins/obsidian/src/store.rs(1 hunks)plugins/obsidian/tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (3)
plugins/obsidian/src/commands.rs (2)
plugins/obsidian/js/bindings.gen.ts (1)
ping(10-12)plugins/obsidian/src/ext.rs (2)
ping(5-5)ping(13-15)
plugins/obsidian/src/ext.rs (2)
plugins/obsidian/src/commands.rs (1)
ping(5-7)plugins/obsidian/js/bindings.gen.ts (1)
ping(10-12)
plugins/obsidian/src/lib.rs (3)
plugins/obsidian/js/bindings.gen.ts (2)
commands(9-13)ping(10-12)plugins/obsidian/src/commands.rs (1)
ping(5-7)plugins/obsidian/src/ext.rs (2)
ping(5-5)ping(13-15)
🪛 markdownlint-cli2 (0.17.2)
plugins/obsidian/permissions/autogenerated/reference.md
5-5: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci
🔇 Additional comments (9)
plugins/obsidian/js/index.ts (1)
1-2: Looks good — concise re-export fulfils guidelineNo unused imports, no error handling, and comment-free as per repo standards. 👍
plugins/obsidian/permissions/autogenerated/commands/ping.toml (1)
5-13: Autogen file looks correctIdentifiers, descriptions, and allow/deny scopes match the command. Nothing further.
plugins/obsidian/build.rs (1)
1-5: LGTM! Standard Tauri plugin build script.The build script correctly registers the "ping" command and follows the expected tauri_plugin::Builder pattern.
plugins/obsidian/package.json (1)
1-12: LGTM! Well-structured package configuration.The package.json properly configures the Obsidian plugin with appropriate dependencies and scripts.
plugins/obsidian/src/commands.rs (1)
1-7: LGTM! Properly implemented Tauri command.The ping command is correctly annotated and implemented, calling the trait method from ObsidianPluginExt. The function signature aligns with the TypeScript bindings.
plugins/obsidian/permissions/autogenerated/reference.md (1)
1-43: Autogenerated permissions documentation looks correct.The permission documentation accurately describes the ping command permissions. Note that static analysis flags some markdown formatting issues, but since this is autogenerated content, manual fixes would be overwritten on regeneration.
plugins/obsidian/src/store.rs (1)
1-9: LGTM! Well-designed store key enum.The StoreKey enum appropriately defines configuration keys with necessary trait derivations for serialization, type reflection, and store integration.
plugins/obsidian/src/lib.rs (1)
29-41: Well-structured plugin initialization.The plugin setup follows Tauri best practices with proper state management and command registration.
plugins/obsidian/js/bindings.gen.ts (1)
1-85: Generated code looks appropriate.This is auto-generated code from tauri-specta, so any issues should be addressed in the generator rather than this file. The TypeScript bindings properly expose the
pingcommand and provide a comprehensive event handling system.
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugins/obsidian/src/ext.rs (1)
14-16: The unwrap() usage issue was previously identified.This is the same unwrap() usage that was flagged in previous reviews as potentially problematic for causing panics.
🧹 Nitpick comments (5)
plugins/obsidian/src/commands.rs (5)
3-9: Remove unnecessaryasyncmodifier.The function is marked as
asyncbut doesn't perform any asynchronous operations. The underlyingis_configured()method returns a regularResult, not aFuture.-pub(crate) async fn is_configured<R: tauri::Runtime>( +pub(crate) fn is_configured<R: tauri::Runtime>(
11-18: Remove unnecessaryasyncmodifier and consider input validation.The function is marked as
asyncbut doesn't perform any asynchronous operations. Additionally, consider validating the API key format to prevent storing invalid values.-pub(crate) async fn set_api_key<R: tauri::Runtime>( +pub(crate) fn set_api_key<R: tauri::Runtime>( app: tauri::AppHandle<R>, api_key: String, ) -> Result<(), String> { + if api_key.trim().is_empty() { + return Err("API key cannot be empty".to_string()); + } app.set_api_key(api_key).map_err(|e| e.to_string()) }
20-27: Remove unnecessaryasyncmodifier and consider URL validation.The function is marked as
asyncbut doesn't perform any asynchronous operations. Additionally, consider validating the base URL format to prevent storing invalid URLs.-pub(crate) async fn set_base_url<R: tauri::Runtime>( +pub(crate) fn set_base_url<R: tauri::Runtime>( app: tauri::AppHandle<R>, base_url: String, ) -> Result<(), String> { + if base_url.trim().is_empty() { + return Err("Base URL cannot be empty".to_string()); + } + if !base_url.starts_with("http://") && !base_url.starts_with("https://") { + return Err("Base URL must start with http:// or https://".to_string()); + } app.set_base_url(base_url).map_err(|e| e.to_string()) }
29-35: Remove unnecessaryasyncmodifier.The function is marked as
asyncbut doesn't perform any asynchronous operations. The underlyingget_api_key()method returns a regularResult, not aFuture.-pub(crate) async fn get_api_key<R: tauri::Runtime>( +pub(crate) fn get_api_key<R: tauri::Runtime>(
37-43: Remove unnecessaryasyncmodifier.The function is marked as
asyncbut doesn't perform any asynchronous operations. The underlyingget_base_url()method returns a regularResult, not aFuture.-pub(crate) async fn get_base_url<R: tauri::Runtime>( +pub(crate) fn get_base_url<R: tauri::Runtime>(
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
Cargo.toml(1 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/capabilities/default.json(1 hunks)apps/desktop/src-tauri/src/lib.rs(1 hunks)apps/desktop/src/components/toolbar/buttons/share-button.tsx(4 hunks)plugins/obsidian/build.rs(1 hunks)plugins/obsidian/js/bindings.gen.ts(1 hunks)plugins/obsidian/permissions/autogenerated/commands/get_api_key.toml(1 hunks)plugins/obsidian/permissions/autogenerated/commands/get_base_url.toml(1 hunks)plugins/obsidian/permissions/autogenerated/commands/is_configured.toml(1 hunks)plugins/obsidian/permissions/autogenerated/commands/set_api_key.toml(1 hunks)plugins/obsidian/permissions/autogenerated/commands/set_base_url.toml(1 hunks)plugins/obsidian/permissions/autogenerated/reference.md(1 hunks)plugins/obsidian/permissions/schemas/schema.json(1 hunks)plugins/obsidian/src/commands.rs(1 hunks)plugins/obsidian/src/error.rs(1 hunks)plugins/obsidian/src/ext.rs(1 hunks)plugins/obsidian/src/lib.rs(1 hunks)
✅ Files skipped from review due to trivial changes (9)
- apps/desktop/package.json
- plugins/obsidian/permissions/autogenerated/commands/is_configured.toml
- plugins/obsidian/permissions/autogenerated/commands/set_api_key.toml
- plugins/obsidian/permissions/autogenerated/commands/get_api_key.toml
- plugins/obsidian/permissions/autogenerated/commands/set_base_url.toml
- Cargo.toml
- apps/desktop/src-tauri/Cargo.toml
- apps/desktop/src-tauri/src/lib.rs
- plugins/obsidian/permissions/autogenerated/commands/get_base_url.toml
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/desktop/src-tauri/capabilities/default.json
- plugins/obsidian/permissions/schemas/schema.json
- plugins/obsidian/src/lib.rs
- apps/desktop/src/components/toolbar/buttons/share-button.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (2)
plugins/obsidian/src/ext.rs (1)
plugins/obsidian/src/commands.rs (5)
is_configured(5-9)set_api_key(13-18)set_base_url(22-27)get_api_key(31-35)get_base_url(39-43)
plugins/obsidian/src/commands.rs (1)
plugins/obsidian/src/ext.rs (10)
is_configured(6-6)is_configured(22-27)set_api_key(7-7)set_api_key(29-34)set_base_url(8-8)set_base_url(36-41)get_api_key(9-9)get_api_key(43-47)get_base_url(10-10)get_base_url(49-53)
🪛 markdownlint-cli2 (0.17.2)
plugins/obsidian/permissions/autogenerated/reference.md
5-5: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (5)
plugins/obsidian/build.rs (1)
1-11: LGTM! Clean and correct build script implementation.The build script properly registers all plugin commands using the standard Tauri plugin builder pattern. The command names match the implementations in the commands module.
plugins/obsidian/permissions/autogenerated/reference.md (1)
1-147: Auto-generated documentation looks correct.The permission documentation follows the standard format for Tauri plugins. The static analysis flags some markdown formatting issues, but these are likely inherent to the documentation generator rather than manual code issues.
plugins/obsidian/src/error.rs (1)
5-18: Well-designed error handling module.The Error enum now has a meaningful variant that wraps store errors, addressing previous concerns about an empty enum. The use of
#[error(transparent)]and#[from]provides clean error propagation, and the Serialize implementation enables proper error serialization for the frontend.plugins/obsidian/js/bindings.gen.ts (1)
9-25: Generated bindings correctly expose the plugin API.The TypeScript bindings properly map to the Rust command implementations with correct parameter types and return types. The async/await pattern aligns with Tauri's command invocation model.
plugins/obsidian/src/ext.rs (1)
3-11: Well-designed trait interface for plugin configuration.The trait provides a clean API for managing Obsidian plugin configuration with appropriate method signatures. The separation between the trait definition and implementation follows good Rust practices.
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.
Actionable comments posted: 2
♻️ Duplicate comments (3)
plugins/obsidian/src/ext.rs (1)
21-23: Remove error handling with unwrap().The
unwrap()call violates the coding guideline of "no error handling" and could cause panics.Based on the coding guidelines, either handle the error properly or remove error handling:
fn obsidian_store(&self) -> tauri_plugin_store2::ScopedStore<R, crate::StoreKey> { - self.scoped_store(crate::PLUGIN_NAME).unwrap() + self.scoped_store(crate::PLUGIN_NAME).expect("Store should be available") }Or change the return type to handle the result:
-fn obsidian_store(&self) -> tauri_plugin_store2::ScopedStore<R, crate::StoreKey>; +fn obsidian_store(&self) -> Option<tauri_plugin_store2::ScopedStore<R, crate::StoreKey>>;apps/desktop/src/components/toolbar/buttons/share-button.tsx (2)
119-121: Remove or implement the Obsidian export option.Offering a non-functional export option creates poor user experience. Users will expect it to work when they click it.
Either remove the Obsidian option from the UI until it's implemented, or implement the actual export functionality. Would you like me to help implement the Obsidian export feature or create an issue to track this?
126-128: Remove error handling per coding guidelines.Error handling violates the project's coding guidelines.
- onError: (error) => { - message(error.message, { title: "Error", kind: "error" }); - },
🧹 Nitpick comments (5)
plugins/store2/permissions/schemas/schema.json (1)
370-374: Schema description updated – stays consistent withdefault.toml.The amended markdown lists the same six granular permissions, keeping the schema self-documenting.
No validation problems detected, and all listed identifiers are present earlier in thePermissionKindenum.If future permissions evolve, repeating the list in two places (here and in
default.toml) risks divergence. You may want to generate this block from a single source of truth during docs build to avoid manual sync errors.plugins/membership/permissions/schemas/schema.json (1)
295-316: Schema can be simplified – replace verboseoneOfblocks with a compactenumUsing three nearly-identical objects inside
oneOfadds noise and maintenance overhead.
JSON-Schema supports anenumarray which conveys the exact same constraint:- "PermissionKind": { - "type": "string", - "oneOf": [ - { "description": "...", "type": "string", "const": "allow-check", "markdownDescription": "..." }, - { "description": "...", "type": "string", "const": "deny-check", "markdownDescription": "..." }, - { "description": "...", "type": "string", "const": "default", "markdownDescription": "..." } - ] - } + "PermissionKind": { + "type": "string", + "enum": ["allow-check", "deny-check", "default"], + "description": "Kinds of primitive permissions the membership plugin understands." + }This keeps semantics unchanged while reducing ~40 lines to ~6 and avoids duplicated
type/constkeys.apps/desktop/src/components/settings/components/types.ts (1)
24-24: Consider using a distinct icon for better UX.Both "feedback" and "integrations" tabs use the same
MessageSquareicon, which could confuse users. Consider using a more specific icon for integrations likePlugorLink.- { name: "integrations", icon: MessageSquare }, + { name: "integrations", icon: Plug },Don't forget to import
Plugfrom "lucide-react".plugins/obsidian/permissions/autogenerated/reference.md (1)
5-5: Fix markdown formatting issues.The heading structure violates markdown best practices:
- Uses h4 (
####) when h3 (###) is expected after h2- Contains trailing punctuation (
:) which should be removedApply this diff to fix the formatting:
-#### This default permission set includes the following: +### This default permission set includes the followingapps/desktop/src/components/settings/views/integrations.tsx (1)
98-98: Remove "what" comments per coding guidelines.Comments should explain "why", not "what". These comments describe what the code does rather than why it's needed.
- {/* Obsidian Integration Card */} - {/* Placeholder for future integrations */}Also applies to: 195-195
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
apps/desktop/src/components/settings/components/tab-icon.tsx(2 hunks)apps/desktop/src/components/settings/components/types.ts(2 hunks)apps/desktop/src/components/settings/views/index.ts(1 hunks)apps/desktop/src/components/settings/views/integrations.tsx(1 hunks)apps/desktop/src/components/toolbar/buttons/share-button.tsx(4 hunks)apps/desktop/src/routes/app.settings.tsx(4 hunks)plugins/membership/permissions/autogenerated/commands/ping.toml(0 hunks)plugins/membership/permissions/autogenerated/reference.md(1 hunks)plugins/membership/permissions/default.toml(1 hunks)plugins/membership/permissions/schemas/schema.json(1 hunks)plugins/obsidian/build.rs(1 hunks)plugins/obsidian/js/bindings.gen.ts(1 hunks)plugins/obsidian/permissions/autogenerated/commands/get_enabled.toml(1 hunks)plugins/obsidian/permissions/autogenerated/commands/get_vault_name.toml(1 hunks)plugins/obsidian/permissions/autogenerated/commands/set_enabled.toml(1 hunks)plugins/obsidian/permissions/autogenerated/commands/set_vault_name.toml(1 hunks)plugins/obsidian/permissions/autogenerated/reference.md(1 hunks)plugins/obsidian/permissions/default.toml(1 hunks)plugins/obsidian/permissions/schemas/schema.json(1 hunks)plugins/obsidian/src/commands.rs(1 hunks)plugins/obsidian/src/ext.rs(1 hunks)plugins/obsidian/src/lib.rs(1 hunks)plugins/obsidian/src/store.rs(1 hunks)plugins/store2/permissions/autogenerated/commands/ping.toml(0 hunks)plugins/store2/permissions/autogenerated/reference.md(1 hunks)plugins/store2/permissions/default.toml(1 hunks)plugins/store2/permissions/schemas/schema.json(1 hunks)plugins/task/build.rs(1 hunks)plugins/task/permissions/autogenerated/commands/ping.toml(0 hunks)plugins/task/permissions/autogenerated/reference.md(0 hunks)plugins/task/permissions/default.toml(1 hunks)plugins/task/permissions/schemas/schema.json(1 hunks)
💤 Files with no reviewable changes (4)
- plugins/store2/permissions/autogenerated/commands/ping.toml
- plugins/task/permissions/autogenerated/commands/ping.toml
- plugins/membership/permissions/autogenerated/commands/ping.toml
- plugins/task/permissions/autogenerated/reference.md
✅ Files skipped from review due to trivial changes (9)
- plugins/membership/permissions/default.toml
- plugins/obsidian/permissions/autogenerated/commands/get_enabled.toml
- plugins/obsidian/permissions/autogenerated/commands/get_vault_name.toml
- apps/desktop/src/components/settings/components/tab-icon.tsx
- plugins/membership/permissions/autogenerated/reference.md
- plugins/obsidian/permissions/autogenerated/commands/set_vault_name.toml
- plugins/obsidian/permissions/autogenerated/commands/set_enabled.toml
- apps/desktop/src/components/settings/views/index.ts
- plugins/store2/permissions/autogenerated/reference.md
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/obsidian/src/lib.rs
- plugins/obsidian/permissions/schemas/schema.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)
Learnt from: plyght
PR: fastrepl/hyprnote#921
File: plugins/location-connectivity/src/error.rs:1-41
Timestamp: 2025-06-06T16:31:46.457Z
Learning: In the location-connectivity plugin (plugins/location-connectivity/), comprehensive error handling via LocationConnectivityError is necessary despite the "no error handling" guideline, as the plugin performs system-level operations (WiFi detection, persistent storage) that require proper error propagation to the frontend.
🧬 Code Graph Analysis (3)
plugins/obsidian/src/ext.rs (1)
plugins/obsidian/src/commands.rs (8)
get_api_key(5-9)set_api_key(13-18)get_base_url(22-26)set_base_url(30-35)get_vault_name(39-43)set_vault_name(47-52)get_enabled(56-60)set_enabled(64-69)
plugins/obsidian/js/bindings.gen.ts (1)
apps/desktop/src/client.ts (1)
baseUrl(10-12)
plugins/obsidian/src/commands.rs (1)
plugins/obsidian/src/ext.rs (16)
get_api_key(7-7)get_api_key(32-36)set_api_key(8-8)set_api_key(38-43)get_base_url(10-10)get_base_url(45-49)set_base_url(11-11)set_base_url(51-56)get_vault_name(13-13)get_vault_name(58-62)set_vault_name(14-14)set_vault_name(64-69)get_enabled(16-16)get_enabled(71-75)set_enabled(17-17)set_enabled(77-82)
🪛 markdownlint-cli2 (0.17.2)
plugins/obsidian/permissions/autogenerated/reference.md
5-5: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (12)
plugins/task/build.rs (1)
1-1: LGTM! Ping command removal correctly implemented.The empty COMMANDS array properly removes the "ping" command as part of the coordinated cleanup effort across plugins.
plugins/task/permissions/schemas/schema.json (1)
298-301: LGTM! Permission schema correctly updated.The PermissionKind description and markdownDescription have been properly updated to reflect the removal of ping-related permissions, leaving only the "default" permission kind.
plugins/task/permissions/default.toml (1)
3-3: LGTM! Default permissions correctly updated.The empty permissions array properly removes the "allow-ping" permission, maintaining consistency with the ping command removal across the plugin.
plugins/store2/permissions/default.toml (1)
3-3: No staleallow-pingreferences found – ready to mergeRan a repository-wide grep for
allow-ping,deny-ping, andping.tomland confirmed no lingering references. The new granular get/set permissions cleanly replace the oldallow-ping.
All set to approve this change.plugins/membership/permissions/schemas/schema.json (1)
295-316: All removed PermissionKinds (allow-ping/deny-ping) have been fully cleared
I scanned the repo for both quoted and unquoted references toallow-pinganddeny-pingand found no occurrences. This change is safe to merge.plugins/obsidian/permissions/default.toml (1)
1-14: LGTM! Well-structured permissions configuration.The permissions list correctly covers all the plugin's commands for API key, base URL, vault name, and enabled status management. The TOML structure follows standard conventions.
apps/desktop/src/routes/app.settings.tsx (2)
8-17: LGTM! Clean import addition.The Integrations component import follows the existing pattern and integrates well with the other view imports.
59-60: Consistent integration of the new tab.The integrations tab is properly integrated across all necessary locations: title mapping, sidebar rendering, and content rendering. The implementation follows the established pattern perfectly.
Also applies to: 107-108, 141-141
plugins/obsidian/src/store.rs (1)
1-12: Well-designed store key implementation.The StoreKey enum is properly structured with appropriate derive macros and correctly implements the ScopedStoreKey trait. The four variants (Enabled, VaultName, BaseUrl, ApiKey) cover the expected configuration options for the Obsidian plugin.
plugins/obsidian/js/bindings.gen.ts (1)
1-107: LGTM!This generated TypeScript bindings file follows the expected patterns for Tauri plugin commands and provides proper type safety for the frontend integration.
apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)
21-27: Well-structured refactoring for multiple export options.The introduction of the
ExportCardinterface and structured export options provides a clean, extensible foundation for supporting multiple export types. The UI design with expandable cards and help links enhances user experience.Also applies to: 38-60
plugins/obsidian/src/commands.rs (1)
1-69: LGTM! Well-structured Tauri command interface.The implementation follows excellent patterns:
- Consistent structure across all 8 commands
- Proper Tauri annotations for command registration and schema generation
- Appropriate error handling with conversion to serializable strings
- Clean separation of concerns as thin wrappers around trait methods
The async signatures align with Tauri requirements, and the
pub(crate)visibility is appropriate for command functions.
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.
Actionable comments posted: 0
♻️ Duplicate comments (3)
plugins/obsidian/src/ext.rs (1)
23-25: Remove error handling per coding guidelines.The
unwrap()call violates the project's coding guidelines which specify "No error handling" for Rust files.Apply this diff to remove error handling:
-fn obsidian_store(&self) -> tauri_plugin_store2::ScopedStore<R, crate::StoreKey> { - self.scoped_store(crate::PLUGIN_NAME).unwrap() -} +fn obsidian_store(&self) -> tauri_plugin_store2::ScopedStore<R, crate::StoreKey> { + self.scoped_store(crate::PLUGIN_NAME).expect("Store should be available") +}apps/desktop/src/components/toolbar/buttons/share-button.tsx (2)
136-138: Remove error handling per coding guidelines.Error handling violates the project's coding guidelines.
-onError: (error) => { - message(error.message, { title: "Error", kind: "error" }); -},
105-131: Fix contradictory Obsidian implementation.The code successfully calls
obsidianCommands.getDeepLinkUrl()but then immediately shows an error message saying "Obsidian export is not yet supported". This creates a confusing user experience.Either implement the full Obsidian functionality or remove it from the UI:
} else if (optionId === "obsidian") { const url = await obsidianCommands.getDeepLinkUrl(session.title); result = { type: "obsidian", url }; }And in the success handler:
-} else if (result?.type === "obsidian") { - message("Obsidian export is not yet supported", { title: "Error", kind: "error" }); -} +} else if (result?.type === "obsidian") { + openUrl(result.url); +}
🧹 Nitpick comments (1)
plugins/obsidian/permissions/autogenerated/reference.md (1)
5-5: Fix markdown formatting issues.The static analysis tools identified formatting issues that should be addressed:
- Heading level jumps from h2 to h4 (should increment by one level)
- Trailing punctuation in heading
Apply this diff to fix the formatting:
-#### This default permission set includes the following: +### This default permission set includes the following
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/obsidian/generated/@tanstack/react-query.gen.tsis excluded by!**/generated/**packages/obsidian/generated/sdk.gen.tsis excluded by!**/generated/**packages/obsidian/generated/types.gen.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
apps/desktop/src/components/toolbar/buttons/share-button.tsx(3 hunks)packages/obsidian/openapi.yaml(14 hunks)packages/obsidian/package.json(1 hunks)plugins/obsidian/build.rs(1 hunks)plugins/obsidian/js/bindings.gen.ts(1 hunks)plugins/obsidian/permissions/autogenerated/commands/get_deep_link_url.toml(1 hunks)plugins/obsidian/permissions/autogenerated/reference.md(1 hunks)plugins/obsidian/permissions/default.toml(1 hunks)plugins/obsidian/permissions/schemas/schema.json(1 hunks)plugins/obsidian/src/commands.rs(1 hunks)plugins/obsidian/src/error.rs(1 hunks)plugins/obsidian/src/ext.rs(1 hunks)plugins/obsidian/src/lib.rs(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- plugins/obsidian/permissions/autogenerated/commands/get_deep_link_url.toml
- packages/obsidian/package.json
- plugins/obsidian/js/bindings.gen.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/obsidian/permissions/default.toml
- plugins/obsidian/build.rs
- plugins/obsidian/src/lib.rs
- plugins/obsidian/permissions/schemas/schema.json
- plugins/obsidian/src/commands.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)
Learnt from: plyght
PR: fastrepl/hyprnote#921
File: plugins/location-connectivity/src/error.rs:1-41
Timestamp: 2025-06-06T16:31:46.457Z
Learning: In the location-connectivity plugin (plugins/location-connectivity/), comprehensive error handling via LocationConnectivityError is necessary despite the "no error handling" guideline, as the plugin performs system-level operations (WiFi detection, persistent storage) that require proper error propagation to the frontend.
🧬 Code Graph Analysis (1)
apps/desktop/src/components/toolbar/buttons/share-button.tsx (4)
apps/desktop/src/contexts/hypr.tsx (1)
useHypr(48-54)packages/utils/src/contexts/sessions.tsx (1)
useSession(49-74)plugins/db/js/bindings.gen.ts (1)
Session(165-165)apps/desktop/src/components/toolbar/utils/pdf-export.ts (1)
exportToPDF(158-342)
🪛 markdownlint-cli2 (0.17.2)
plugins/obsidian/permissions/autogenerated/reference.md
5-5: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (5)
plugins/obsidian/permissions/autogenerated/reference.md (1)
1-260: LGTM! Comprehensive permission documentation.The permission reference provides clear documentation of all available commands with both allow and deny variants, making it easy for developers to understand the plugin's permission model.
plugins/obsidian/src/error.rs (1)
1-21: LGTM! Error handling properly implemented.The Error enum now has meaningful variants and the Serialize implementation enables proper error communication across the Tauri boundary. This addresses the previous issues with the empty enum.
plugins/obsidian/src/ext.rs (1)
27-97: LGTM! Well-structured trait implementation.The trait methods provide a clean interface for managing Obsidian plugin configuration with proper store operations and error propagation patterns.
apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)
22-84: LGTM! Well-structured component with good UX improvements.The refactored interface with expandable cards, proper analytics tracking, and conditional Obsidian integration provides a much better user experience than the previous implementation.
packages/obsidian/openapi.yaml (1)
1-2081: LGTM! Comprehensive API documentation.The OpenAPI specification is well-structured and provides comprehensive documentation for the Obsidian Local REST API. The additions of system endpoints and expanded periodic notes functionality with full CRUD operations enhance the API's capabilities while maintaining clear documentation standards.
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.
Actionable comments posted: 1
♻️ Duplicate comments (3)
plugins/obsidian/build.rs (1)
1-13: Add the missing "is_configured" command to the build script.apps/desktop/src/components/settings/views/integrations.tsx (1)
82-93: Add missing dependencies to useEffect.plugins/obsidian/src/ext.rs (1)
26-28: Remove error handling with unwrap().This is the same issue flagged in previous reviews. The
unwrap()call violates the coding guideline of "no error handling" and could cause panics.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/desktop/src/components/settings/views/integrations.tsx(1 hunks)apps/desktop/src/components/toolbar/buttons/share-button.tsx(3 hunks)packages/obsidian/index.ts(1 hunks)plugins/obsidian/build.rs(1 hunks)plugins/obsidian/js/bindings.gen.ts(1 hunks)plugins/obsidian/permissions/autogenerated/commands/get_base_folder.toml(1 hunks)plugins/obsidian/permissions/autogenerated/commands/set_base_folder.toml(1 hunks)plugins/obsidian/permissions/autogenerated/reference.md(1 hunks)plugins/obsidian/permissions/default.toml(1 hunks)plugins/obsidian/permissions/schemas/schema.json(1 hunks)plugins/obsidian/src/commands.rs(1 hunks)plugins/obsidian/src/ext.rs(1 hunks)plugins/obsidian/src/lib.rs(1 hunks)plugins/obsidian/src/store.rs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- plugins/obsidian/permissions/autogenerated/commands/get_base_folder.toml
- plugins/obsidian/permissions/autogenerated/commands/set_base_folder.toml
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/obsidian/permissions/default.toml
- plugins/obsidian/src/lib.rs
- plugins/obsidian/permissions/schemas/schema.json
- apps/desktop/src/components/toolbar/buttons/share-button.tsx
- plugins/obsidian/js/bindings.gen.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (3)
apps/desktop/src/components/settings/views/integrations.tsx (1)
plugins/obsidian/js/bindings.gen.ts (6)
getEnabled(28-30)getApiKey(16-18)getBaseUrl(19-21)commands(9-43)setApiKey(10-12)setBaseUrl(13-15)
plugins/obsidian/src/ext.rs (1)
plugins/obsidian/src/commands.rs (11)
get_api_key(5-9)set_api_key(13-18)get_base_url(22-26)set_base_url(30-35)get_vault_name(39-43)set_vault_name(47-52)get_base_folder(56-60)set_base_folder(64-69)get_enabled(73-77)set_enabled(81-86)get_deep_link_url(90-95)
plugins/obsidian/src/commands.rs (1)
plugins/obsidian/src/ext.rs (22)
get_api_key(7-7)get_api_key(37-41)set_api_key(8-8)set_api_key(43-48)get_base_url(10-10)get_base_url(50-54)set_base_url(11-11)set_base_url(56-61)get_vault_name(13-13)get_vault_name(63-67)set_vault_name(14-14)set_vault_name(69-74)get_base_folder(16-16)get_base_folder(76-80)set_base_folder(17-17)set_base_folder(82-87)get_enabled(19-19)get_enabled(89-93)set_enabled(20-20)set_enabled(95-100)get_deep_link_url(22-22)get_deep_link_url(102-112)
🪛 markdownlint-cli2 (0.17.2)
plugins/obsidian/permissions/autogenerated/reference.md
5-5: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (6)
packages/obsidian/index.ts (1)
1-2: LGTM! Clean export reorganization.The transition from external dependency to generated local modules aligns well with the new Obsidian plugin infrastructure.
plugins/obsidian/src/store.rs (1)
1-12: LGTM! Well-structured store key implementation.The enum variants appropriately represent the configuration keys needed for the Obsidian integration, and the trait implementations are correct.
plugins/obsidian/permissions/autogenerated/reference.md (1)
1-314: Comprehensive permission documentation generated correctly.The autogenerated permission reference accurately documents all command permissions for the Obsidian plugin integration.
plugins/obsidian/src/ext.rs (2)
3-23: LGTM - Clean trait definition.The trait interface is well-designed with clear method signatures for configuration management.
89-93: Remove error handling with unwrap_or().The
unwrap_or(false)call violates the coding guideline of "no error handling". Consider changing the approach to avoid error handling.fn get_enabled(&self) -> Result<bool, crate::Error> { let store = self.obsidian_store(); - let v = store.get::<bool>(crate::StoreKey::Enabled)?; - Ok(v.unwrap_or(false)) + store.get::<bool>(crate::StoreKey::Enabled) + .map(|v| v.unwrap_or(false)) }Likely an incorrect or invalid review comment.
plugins/obsidian/src/commands.rs (1)
1-95: LGTM - Well-structured Tauri commands.The commands follow consistent patterns and proper error conversion without panic-inducing operations. The implementation correctly wraps the trait methods for Tauri frontend integration.
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.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)
188-191: Remove error handling per coding guidelines.Error handling violates the project's coding guidelines.
- onError: (error) => { - console.error(error); - message(JSON.stringify(error), { title: "Error", kind: "error" }); - },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/desktop/package.json(2 hunks)apps/desktop/src-tauri/capabilities/default.json(2 hunks)apps/desktop/src/components/settings/views/integrations.tsx(1 hunks)apps/desktop/src/components/toolbar/buttons/share-button.tsx(3 hunks)packages/tiptap/src/shared/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/tiptap/src/shared/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/package.json
- apps/desktop/src-tauri/capabilities/default.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)
Learnt from: plyght
PR: fastrepl/hyprnote#921
File: plugins/location-connectivity/src/error.rs:1-41
Timestamp: 2025-06-06T16:31:46.457Z
Learning: In the location-connectivity plugin (plugins/location-connectivity/), comprehensive error handling via LocationConnectivityError is necessary despite the "no error handling" guideline, as the plugin performs system-level operations (WiFi detection, persistent storage) that require proper error propagation to the frontend.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (3)
apps/desktop/src/components/settings/views/integrations.tsx (1)
88-112: Well-structured form change handler.The useEffect properly watches form changes and calls the appropriate commands based on field names. The implementation correctly handles the subscription cleanup.
apps/desktop/src/components/toolbar/buttons/share-button.tsx (2)
54-78: Well-implemented conditional export options.The export options array correctly includes Obsidian conditionally based on configuration state, and the filtering approach is clean.
96-168: Obsidian export functionality successfully implemented.The export mutation now handles all three export types (PDF, email, Obsidian) with proper result types and processing logic. This addresses the previous concern about non-functional export options.
| baseUrl: z.string().url().refine((url) => url.startsWith("http://") && !url.startsWith("https://"), { | ||
| message: "URL must start with http://, not https://", | ||
| }), |
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.
🛠️ Refactor suggestion
Consider the security implications of enforcing HTTP over HTTPS.
The validation explicitly rejects HTTPS URLs in favor of HTTP. This could expose API keys and sensitive data in transit. Ensure this is intentional for local development scenarios.
🤖 Prompt for AI Agents
In apps/desktop/src/components/settings/views/integrations.tsx around lines 23
to 25, the URL validation currently enforces URLs to start with "http://" and
rejects "https://", which can expose sensitive data. Review the requirement and
if HTTPS should be allowed, modify the validation to accept URLs starting with
"https://" as well, or remove the refine check that excludes HTTPS. If HTTP is
only allowed for local development, add a conditional to permit HTTPS in other
environments.
Screen.Recording.2025-07-14.at.12.00.28.AM.mov