-
Notifications
You must be signed in to change notification settings - Fork 665
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
generate copy without wikilinks #1365
generate copy without wikilinks #1365
Conversation
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.
Thanks for looking into this.
At a high level, have you looked at MarkdownLink.createUpdateLinkEdit
? it should do most of what you need in terms of refactoring.
Can you also tell me your thinking about having this as a command vs a formatting setting (kinda like using wikilink definitions)?
I would like to understand the feature from the POV of the user: is this about having a documenta that is always md compatible (in which case the transformation would happen on save), or is it about being able to export the knowledge base for e.g. a static site generator (in which case do we need a command for that?), or about exporting a single file (which is what you are implementing IIUC) - thoughts?
packages/foam-vscode/src/features/commands/generate-standalone-note.ts
Outdated
Show resolved
Hide resolved
} | ||
return ''; | ||
} | ||
public toMarkdownLink( |
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.
You probably can reuse the mechanisms available in MarkdownLink
to achieve the same in a more integrated way
Thank you for the quick review. Currently, I ran into a bug that it cannot resolve the wikilink to embeded images URI through The reason for doing this is wikilinks are more flexible, which leaves the workspace to manage the resource path. For example, I usually start with a single draft, and with more resources are added in the idea gets mature. At some point, I would like to re-organize or archive a group of ideas. If files are linked by wikilinks, I can move them freely. So, I think keeping linkages as wikilink would be benefit for PKM. However, if choosing format, there could be chance that links get broken, and it will need efforts to fix them. In my imagination, users are recommanded to use wikilinks and only need to generate a standalone copy when they want to "export" their ideas. After that, those copies can be wiped out, by manually or scripts. Single file is not neccessary. Providing options to export a list of selected files or a whole directory would be helpful. |
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.
PR is looking good, I left a few comments. Thanks for the explanation around the rationale, we are aligned and this feels like it's pushing things in the right direction
const targetRes = workspace.find(targetUri); | ||
let relativeUri = targetRes.uri.relativeTo(resource.uri.getDirectory()); | ||
|
||
if (targetFormat === 'wikilink') { | ||
/* remove extension if possible, then get the basename to prevent from filename conflict */ | ||
if (relativeUri.path.endsWith(workspace.defaultExtension)) { | ||
relativeUri = relativeUri.changeExtension('*', ''); | ||
} | ||
target = relativeUri.getBasename(); | ||
|
||
return { | ||
newText: `${embed}[[${target}${sectionDivider}${section}${aliasDivider}${alias}]]`, | ||
range: link.range, | ||
}; | ||
} | ||
|
||
if (targetFormat === 'link') { | ||
/* if alias is empty, construct one as target#section */ | ||
if (alias === '') { | ||
/* in page anchor have no filename */ | ||
if (relativeUri.getBasename() === resource.uri.getBasename()) { | ||
target = ''; | ||
} | ||
alias = `${target}${sectionDivider}${section}`; | ||
} | ||
|
||
/* construct url */ | ||
let url = relativeUri.path; | ||
/* in page anchor have no filename */ | ||
if (relativeUri.getBasename() === resource.uri.getBasename()) { | ||
url = ''; | ||
} | ||
if (sectionDivider === '#') { | ||
url = `${url}${sectionDivider}${section}`; | ||
} | ||
if (url.indexOf(' ') > 0) { | ||
url = `<${url}>`; | ||
} | ||
|
||
/* if it's originally an embeded note, the markdown link shouldn't be embeded */ | ||
if (embed && targetRes.type === 'note') { | ||
embed = ''; | ||
} | ||
return { | ||
newText: `${embed}[${alias}](${url})`, | ||
range: link.range, | ||
}; | ||
} |
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.
I haven't tested it locally, but I wonder if you could be able to replace this whole section with
const newLink = {...link, type: targetType}
const edit = MarkdownLink.createUpdateLinkEdit(newLink)
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.
I think it's impossible. The first line of MarkdownLink.createUpdateLinkEdit
is
const { target, section, alias } = MarkdownLink.analyzeLink(link);
which extracts three components via regex from link.rawText based on link.type. If the pattern doesn't match, three of them may not be extracted correctly.
Moreover, when dealing with conversion from wikilink to link, we have to retrieve the relative path to target, which is usually a basename in wikilink's rawText. Therefore, we must have workspace
and note: Resource | URI
.
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.
Yeah, good point. Basically instead increateUpdateLinkEdit
we need to add type?: 'wikilink' | 'link'
to the delta
parameter, and take it from there. Does that sound good?
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.
Here is one possible version. I added type
and isEmbed
to the delta
, which slightly affacts how createUpdateLinkEdit
composes newText. Please check it.
packages/foam-vscode/src/features/commands/convert-links-format-in-note.ts
Outdated
Show resolved
Hide resolved
/** | ||
* convert links based on its workspace and the note containing it. | ||
* Changes happen in a copy | ||
* 1. prepare a copy file, and makt it the activeTextEditor |
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.
makt?
const resource = fParser.parse(fromVsCodeUri(doc.uri), text); | ||
|
||
const textReplaceArr = resource.links | ||
.filter(link => link.type === convertOption.from) |
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.
(note: in my proposed approach, this check would be link.type !== convertOption.to
)
Co-authored-by: Riccardo <[email protected]>
…LinkEdit-accomplish-convert modify createUpdateLinkEdit to accomplish convert
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.
Thanks @hereistheusername - it all looks good and I think it's good to go, except for the require
use - can you please see if the proposed solutions work? I am not keen on introducing require
import { ResourceParser } from '../../core/model/note'; | ||
import { IMatcher } from '../../core/services/datastore'; | ||
import { convertLinkFormat } from '../../core/janitor'; | ||
const vscode = require('vscode'); /* cannot import workspace from above statement and not sure what happened */ |
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.
can you try again to import workspace
from vscode, or use import * from vscode
and use that?
I am not introducing the require
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.
Done.
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.
Great, thx!
@allcontributors add @hereistheusername for code |
I've put up a pull request to add @hereistheusername! 🎉 |
* add generate standalone note command * fix embeded wikilinks * refactor convertLinksFormat function & add 4 user command interfaces * change user interface * modify createUpdateLinkEdit to accomplish convert * only images can be embedded * keey filename when using in page anchor * give a default value to alias in link format combination branch * add tests to createUpdateLinkEdit about changint links' type and isEmbed * get target from getIdentifier --------- Co-authored-by: Riccardo <[email protected]>
Abstract
As mentioned in #1353 , #1319, and #791, many people got difficulties when working with other tools that don't support wikilinks. To tackle this problem, I add a function to convert link format.
Further Explanation
Implementation
Refactoring the
MarkdownLink.createUpdateLinkEdit
, theconvertLinkFormat
function resolves links based the note containing it and its workspace and combine components properly.User Interface
4 commands:
to meets users' different needs.
Thanks to @riccardoferretti help.