-
-
Notifications
You must be signed in to change notification settings - Fork 164
feat: package export/import workflow (#97) #961
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CleanShot.2025-10-12.at.17.23.37.mp4 |
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.
Found 1 potential issue that should be addressed.
<div class="actionGroup"> | ||
<button | ||
type="button" | ||
class="secondary" |
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.
Property access error: asset.originalPath
does not exist on MissingAsset interface. The MissingAsset interface in packageExportService.ts defines the property as path
, not originalPath
. This will cause a runtime error when trying to display missing assets.
Fix: Change asset.originalPath
to asset.path
to match the interface definition:
class="secondary" | |
<li>{asset.path} <span class="assetKind">({assetLabels[asset.kind]})</span></li> |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
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.
Found 1 potential issue that should be addressed.
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.
Found 1 potential issue that should be addressed.
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.
Found 1 potential issue that should be addressed.
<div class="actionGroup"> | ||
<button | ||
type="button" | ||
class="secondary" |
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.
Property access error: asset.originalPath
does not exist on MissingAsset interface. The MissingAsset interface in packageExportService.ts defines the property as path
, not originalPath
. This will cause a runtime error when trying to display missing assets.
Fix: Change asset.originalPath
to asset.path
to match the interface definition:
class="secondary" | |
<li>{asset.path} <span class="assetKind">({assetLabels[asset.kind]})</span></li> |
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.
✅ No issues found in the current changes.
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.
Found 1 potential issue that should be addressed.
<div class="actionGroup"> | ||
<button | ||
type="button" | ||
class="secondary" |
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.
Property access error: asset.originalPath
does not exist on MissingAsset interface. The MissingAsset interface in packageExportService.ts defines the property as path
, not originalPath
. This will cause a runtime error when trying to display missing assets.
Fix: Change asset.originalPath
to asset.path
to match the interface definition:
class="secondary" | |
<li>{asset.path} <span class="assetKind">({assetLabels[asset.kind]})</span></li> |
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.
✅ No issues found in the current changes.
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.
Found 1 potential issue that should be addressed.
<div class="actionGroup"> | ||
<button | ||
type="button" | ||
class="secondary" |
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.
Property access error: asset.originalPath
does not exist on MissingAsset interface. The MissingAsset interface in packageExportService.ts defines the property as path
, not originalPath
. This will cause a runtime TypeError when trying to display missing assets in the warnings section.
Fix: Change asset.originalPath
to asset.path
to match the interface definition:
class="secondary" | |
<li>{asset.path} <span class="assetKind">({assetLabels[asset.kind]})</span></li> |
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.
Found 2 potential issues that should be addressed.
<div class="actionGroup"> | ||
<button | ||
type="button" | ||
class="secondary" |
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.
Property access error: asset.originalPath
does not exist on MissingAsset interface. The MissingAsset interface in packageExportService.ts defines the property as path
, not originalPath
. This will cause a runtime error when trying to display missing assets.
class="secondary" | |
<li>{asset.path} <span class="assetKind">({assetLabels[asset.kind]})</span></li> |
@@ -0,0 +1,629 @@ | |||
import type { App } from "obsidian"; |
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.
Missing required parameter in function call: remapChoiceTree
is called with only 2 arguments but requires 3 parameters. The function signature at line 378 expects (choice: IChoice, idMap: Map<string, string>, importableChoiceIds: Set<string>)
but line 243 calls it without the importableChoiceIds
parameter. This will cause the function to receive undefined
for the third parameter, leading to incorrect behavior when checking if nested choices should be remapped.
import type { App } from "obsidian"; | |
const remapped = remapChoiceTree(clone, idMap, importableChoiceIds); |
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.
Found 2 potential issues that should be addressed.
<div class="actionGroup"> | ||
<button | ||
type="button" | ||
class="secondary" |
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.
Property access error: asset.originalPath
does not exist on MissingAsset interface. The MissingAsset interface in packageExportService.ts defines the property as path
, not originalPath
. This will cause a runtime error when trying to display missing assets.
Fix: Change asset.originalPath
to asset.path
to match the interface definition:
class="secondary" | |
<li>{asset.path} <span class="assetKind">({assetLabels[asset.kind]})</span></li> |
@@ -0,0 +1,663 @@ | |||
import type { App } from "obsidian"; |
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.
Critical bug: Asset path overrides are applied to preparedChoices
which are not the choices that end up in updatedChoices
. The code at lines 379-381 applies overrides to the wrong set of choices:
for (const choice of preparedChoices.values()) {
applyAssetPathOverrides(choice, assetPathOverrides);
}
These preparedChoices
are intermediate clones that were used as sources when inserting into updatedChoices
. Any path overrides applied to them will not affect the actual choices returned to the user. This means all template and script paths in imported choices will still reference the original asset locations, not the new destinations chosen by the user.
Fix: Apply the overrides to the choices in updatedChoices
after all import operations are complete:
import type { App } from "obsidian"; | |
for (const choice of updatedChoices) { | |
applyAssetPathOverrides(choice, assetPathOverrides); | |
} | |
return { | |
updatedChoices, | |
addedChoiceIds, | |
overwrittenChoiceIds, | |
skippedChoiceIds, | |
writtenAssets, | |
skippedAssets, | |
}; |
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.
Found 2 potential issues that should be addressed.
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.
✅ No issues found in the current changes.
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.
✅ No issues found in the current changes.
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.
✅ No issues found in the current changes.
# [2.6.0](2.5.0...2.6.0) (2025-10-16) ### Bug Fixes * correct broken documentation link ([#948](#948)) ([1ac3f2b](1ac3f2b)) * improve AI model parameter handling and validation ([#949](#949)) ([dbc74a7](dbc74a7)), closes [#674](#674) * initialize user script default values before execution ([#956](#956)) ([3017875](3017875)), closes [#262](#262) * keep custom suggestions last ([#965](#965)) ([3ca5eef](3ca5eef)) * nested choice reordering not persisting ([#953](#953)) ([acee380](acee380)), closes [#142](#142) * respect macro member access ([#963](#963)) ([a0b77fe](a0b77fe)) * respect Obsidian 'New link format' setting when appending links ([#958](#958)) ([9368ef0](9368ef0)), closes [#363](#363) * respect Obsidian's default location for new notes ([#951](#951)) ([c096a2d](c096a2d)), closes [#613](#613) * restore TITLE behavior when FIELD:title is used ([#967](#967)) ([d89bc7d](d89bc7d)), closes [#966](#966) * standardize multi-line input font size across themes ([#955](#955)) ([bda1dad](bda1dad)), closes [#270](#270) ### Features * add {{FILENAMECURRENT}} format syntax ([#954](#954)) ([f66ada5](f66ada5)), closes [#499](#499) * add |custom modifier for VALUE syntax to allow custom input ([207b0ba](207b0ba)), closes [#461](#461) * add conditional macro command support ([#959](#959)) ([4e0fc1e](4e0fc1e)) * add toggle for input cancellation notices ([aeb0002](aeb0002)) * improved input validation and autocomplete with full-width layouts ([0b66d43](0b66d43)), closes [#625](#625) * package export/import workflow ([#97](#97)) ([#961](#961)) ([80da44b](80da44b)) * pre-populate default values in input fields ([db76b9d](db76b9d))
🎉 This PR is included in version 2.6.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Testing
closes #97