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

Rudimentary way to link to files. #122

Closed
wants to merge 6 commits into from
Closed

Conversation

Mogyula
Copy link

@Mogyula Mogyula commented Apr 8, 2023

Hi!

This is my very first pull request sent to any project ever; I hope this is how it works. :)

So it was when I deployed a Digital Garden based on an Obsidian vault that references a ton of locally saved PDFs (as to support ideation for a reasearch project) that I realized that these wouldn't work online, as the plugin wouldn't commit ordinary files, only notes and images I guess.

Now I didn't do any work on the plugin, but I came to realize that if I just commit those PDFs manually, and also apply these changes to the codebase, then they'd be accessible.

The change on the regexp in .eleventy.js:160 is to cosnider links with or without the pipeline symbol, because I've noticed that notes' links get converted to the labeled format as in [[Another page|Another page]] even if in Obsidian it was just [[Another page]]. With PDFs though, you get [[dummy.pdf]] without the pipe.

See this site for a demo.

Gyula

@oleeskild
Copy link
Owner

Very cool, congrats on your first PR! I'll have a look over it some time this week.

Copy link
Owner

@oleeskild oleeskild left a comment

Choose a reason for hiding this comment

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

Sorry for being so slow reviewing this. I've added some comments on things that needs to be changed for this to be able to be merged in.

.eleventy.js Outdated
let headerLinkPath;


if (fileLink.slice(-5).includes(".") && fileLink.slice(-3)!=".md"){
Copy link
Owner

Choose a reason for hiding this comment

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

Why does slice(-5).includes(".") imply its a file? This seems to only work with filetypes that have an extension that is smaller than 5 characters.

Copy link
Owner

Choose a reason for hiding this comment

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

This file need to be removed before this PR can be merged.

Copy link
Owner

Choose a reason for hiding this comment

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

This file need to be removed before this PR can be merged.

Copy link
Owner

Choose a reason for hiding this comment

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

This file need to be removed before this PR can be merged.

@Mogyula Mogyula requested a review from oleeskild June 10, 2023 20:14
@Mogyula
Copy link
Author

Mogyula commented Jun 10, 2023

Sorry on my part as well for getting back to this with such a delay. I believe the requested changes have been made. Let me know otherwise.

@oleeskild oleeskild closed this Aug 14, 2024
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