Skip to content

Conversation

rekby
Copy link

@rekby rekby commented Jan 27, 2024

Let's add copy link to a note as markdown, with filename as title.

Copy link
Owner

@Vinzent03 Vinzent03 left a comment

Choose a reason for hiding this comment

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

Interesting idea. What's your use case for this feature? I've left some comments.

// const copyFunc = asMarkdown ?
// (parameters)=>this.tools.copyMarkdownLink(parameters) : (parameters) =>this.tools.copyURI(parameters);

let copyFunc = (parameters: Parameters) => {
Copy link
Owner

Choose a reason for hiding this comment

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

this can be const

const view = app.workspace.getActiveViewOfType(FileView);
if (!view && !file) return;

// const copyFunc = asMarkdown ?
Copy link
Owner

Choose a reason for hiding this comment

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

The comment should be removed.

@@ -116,6 +117,16 @@ export default class Tools {
new Notice("Advanced URI copied to your clipboard");
}

async copyMarkdownLink(parameters: Parameters) {
Copy link
Owner

Choose a reason for hiding this comment

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

the markdownTitle should not be added to Parameters,because Parameters should only be used for uri input or output. Add it as another argument.

const uri = await this.generateURI(parameters, true);
const markdownLink = "[" + markdownTitle + "](" + uri + ")";

new Notice("Advanced URI copied to your clipboard (Markdown)");
Copy link
Owner

Choose a reason for hiding this comment

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

In case the copying fails, the notice should be placed after the actual copy function call.

);
});
menu.addItem((item)=>{
Copy link
Owner

Choose a reason for hiding this comment

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

I feel like the file menu is already pretty full. I would rather keep it concise and therefore not add this item.

Copy link
Author

Choose a reason for hiding this comment

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

I think need both version, because remove old behaviour will break current scenarios.

And it is may be not comfort as option in settings, because somebody can need both types of link and he will deside about link type at copy moment. It depends from target link (as simple link or paste to another markdown file).

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, but I can't follow your comment. Just keep the old menu item with the old function to just copy the uri. The added commands should be used as a way to copy the whole markdown link.

Copy link
Author

@rekby rekby Feb 2, 2024

Choose a reason for hiding this comment

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

Can I add an option to the settings for show/hide markdown item (hidden by default)?

@@ -63,6 +63,12 @@ export default class AdvancedURI extends Plugin {
callback: () => this.handlers.handleCopyFileURI(true),
});

this.addCommand({
Copy link
Owner

Choose a reason for hiding this comment

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

I would either add the markdown version for the command with options as well, or just the one with options.

@rekby
Copy link
Author

rekby commented Feb 2, 2024

Interesting idea. What's your use case for this feature? I've left some comments.

I need work with two vaults: work and personal. Sometimes I need link from each to other.
When I paste link I want paste it to new vault with original title and don't type in manually.

Thanks for review, I will fix style soon

@Vinzent03 Vinzent03 closed this in 84b3cd8 Aug 26, 2025
@Vinzent03
Copy link
Owner

Thanks for your pr, but I've solved it now differently myself.

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