Skip to content
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

Merged
merged 24 commits into from
Jul 10, 2024

Conversation

hereistheusername
Copy link
Contributor

@hereistheusername hereistheusername commented Jun 21, 2024

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, the convertLinkFormat function resolves links based the note containing it and its workspace and combine components properly.

User Interface

4 commands:

  1. foam-vscode.convert-wikilink-to-markdownlink-inplace
  2. foam-vscode.convert-markdownlink-to-wikilink-inplace
  3. foam-vscode.convert-wikilink-to-markdownlink-in-copy
  4. foam-vscode.convert-markdownlink-to-wikilink-in-copy

to meets users' different needs.

Thanks to @riccardoferretti help.

Copy link
Collaborator

@riccardoferretti riccardoferretti left a 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?

}
return '';
}
public toMarkdownLink(
Copy link
Collaborator

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

@hereistheusername
Copy link
Contributor Author

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?

Thank you for the quick review. Currently, I ran into a bug that it cannot resolve the wikilink to embeded images URI through workspace.resolveLink(resource, link). I am working on it and I'll also check the MarkdownLink.createUpdateLinkEdit.

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.

@hereistheusername hereistheusername marked this pull request as ready for review June 23, 2024 15:19
Copy link
Collaborator

@riccardoferretti riccardoferretti left a 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

Comment on lines 52 to 99
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,
};
}
Copy link
Collaborator

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)

Copy link
Contributor Author

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.

Copy link
Collaborator

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?

Copy link
Contributor Author

@hereistheusername hereistheusername Jun 28, 2024

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.

differ to current code
branch

/**
* 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
Copy link
Collaborator

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)
Copy link
Collaborator

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)

Copy link
Collaborator

@riccardoferretti riccardoferretti left a 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 */
Copy link
Collaborator

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator

@riccardoferretti riccardoferretti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thx!

@riccardoferretti riccardoferretti merged commit cef8d2a into foambubble:master Jul 10, 2024
3 checks passed
@riccardoferretti
Copy link
Collaborator

@allcontributors add @hereistheusername for code

Copy link
Contributor

@riccardoferretti

I've put up a pull request to add @hereistheusername! 🎉

pderaaij pushed a commit to pderaaij/foam that referenced this pull request Jul 14, 2024
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants