Index []() style links

[onespaceman]

• style links
  • No longer opens in new window if link is local
  • Now indexed if local
  • Page completion
  • Changed on rename and refactor
    • Refactor now uses link position from the index to change references
• New query for linked attachments

[zefhemel]

This is very nice! Let me have a closer look at this and do some testing.

[zefhemel]

I've noticed that linking to other pages with the [hello](somepage) only works for pages linked to without spaces, so [hello](some page) doesn't work, because the underlying markdown parser doesn't accept it. I was thinking of different options, but probably the best one would be to patch the markdown parser somehow. Alternatively we just don't support page links with spaces, or support them, but URL encoded (e.g. other%20page) but that doesn't seem great either.

[zefhemel]

Another question is: should attachment links be tagged with attachment or e.g. attachmentlink or attachmentLink. I think it would be useful to reserve attachment (the files themselves) and index them with their own attributes (such as file size, lastModified, contentType etc.)

[onespaceman]

I've noticed that linking to other pages with the [hello](somepage) only works for pages linked to without spaces, so [hello](some page) doesn't work, because the underlying markdown parser doesn't accept it. I was thinking of different options, but probably the best one would be to patch the markdown parser somehow. Alternatively we just don't support page links with spaces, or support them, but URL encoded (e.g. other%20page) but that doesn't seem great either.

Yeah, apparently the correct way to enclose the link with <> if it has whitespaces. It doesn't work right now, but I could add it and call it a day. I'm not a fan, but if the spec says this is how it must be, who am I to argue?

[onespaceman]

Another question is: should attachment links be tagged with attachment or e.g. attachmentlink or attachmentLink. I think it would be useful to reserve attachment (the files themselves) and index them with their own attributes (such as file size, lastModified, contentType etc.)

Good point. another option is to combine with regular LinkObjects as a union

type LinkObject = ObjectValue<{
  // Page Link
  // The page the link points to
  toPage: string;
  // The page the link occurs in
  page: string;
  pos: number;
  snippet: string;
  alias?: string;
  asTemplate: boolean;
  toFile?: never;
} | {
  // Attachment Link
  // The file the link points to
  toFile: string;
  // The page the link occurs in
  page: string;
  pos: number;
  snippet: string;
  toPage?: never;
  alias?: never;
  asTemplate?: never;
}>;

What do you think?

[zefhemel]

I've noticed that linking to other pages with the [hello](somepage) only works for pages linked to without spaces, so [hello](some page) doesn't work, because the underlying markdown parser doesn't accept it. I was thinking of different options, but probably the best one would be to patch the markdown parser somehow. Alternatively we just don't support page links with spaces, or support them, but URL encoded (e.g. other%20page) but that doesn't seem great either.

Yeah, apparently the correct way to enclose the link with <> if it has whitespaces. It doesn't work right now, but I could add it and call it a day. I'm not a fan, but if the spec says this is how it must be, who am I to argue?

Really? First time I heard of that notation. Markdown is a new surprise every day 😆

[zefhemel]

Another question is: should attachment links be tagged with attachment or e.g. attachmentlink or attachmentLink. I think it would be useful to reserve attachment (the files themselves) and index them with their own attributes (such as file size, lastModified, contentType etc.)

Good point. another option is to combine with regular LinkObjects as a union

type LinkObject = ObjectValue<{
  // Page Link
  // The page the link points to
  toPage: string;
  // The page the link occurs in
  page: string;
  pos: number;
  snippet: string;
  alias?: string;
  asTemplate: boolean;
  toFile?: never;
} | {
  // Attachment Link
  // The file the link points to
  toFile: string;
  // The page the link occurs in
  page: string;
  pos: number;
  snippet: string;
  toPage?: never;
  alias?: never;
  asTemplate?: never;
}>;

What do you think?

Actually yeah, that makes sense, let's do that. I suppose you can easily filter one or the other by filtering on either toPage or toFile.

[onespaceman]

While I'm doing this I'm also adding in updating attachment links on rename/refactor (fixes #733)
questions:

1. Should attachment links be changed to be the same as wikilinks where every link assumes it's from the base folder, or should it refer to the note's folder like it is now? ie Page is [[subfolder/Page]]. image.png is inside subfolder. Link right now -> [](image.png). Should it be [](subfolder/image.png)?
2. Should I add support for attachment wikilinks? -> ![[image.png]]

• Should it be the default?
• If added then implementing the change in question 1 makes sense

4. When a Page is moved where do the attachments go?

• Option 1: Only update the links, Keep the attachment where it is.
• Option 2: If the attachment is in the same folder as the page, and is only linked to on that page, move it with the page. Otherwise do option 1

[zefhemel]

As for 1. Right, so the semantics of links and image links should be that they’re relative, to be consistent with default markdown behavior.

As for 2. I think that would be great actually. And making that the default drag & drop etc. behavior… good question. Maybe?

As for 4. Option 2 would be the nicest I’d say

[onespaceman]

Ok, this turned out to be pretty big, so I did a hard reset and tried to break it into digestible commits.
I tried to be as thorough as possible with testing because it involves editing/moving pages so I don't want to mess up anyone's files.

Here is a basic space setup I used for testing how renaming works with rewriting links
https://gist.github.com/onespaceman/da4eaed2e6808e7342840c9f680ce84d
I also used a copy of my personal vault to rename and prefix rename everything and anything, it seems to work perfectly, but please take your time to test it out yourself.

Also I had a really weird bug with importing regex constants.
https://github.com/onespaceman/silverbullet/blob/b17a2d0d468034d5434a88c7102639bf0a4da3d2/web/cm_plugins/inline_image.ts#L67
https://github.com/onespaceman/silverbullet/blob/b17a2d0d468034d5434a88c7102639bf0a4da3d2/web/cm_plugins/inline_image.ts#L70
When using the imported regex, the inline images would only show up every other image, and the visible images would disappear and the not shown images would appear on any refresh, such as moving the cursor or typing a character. This doesn't happen when I would replace the constant by pasting the same regex directly. Idk what is going on there.

[zefhemel]

Ok let me give this a try. We clearly need an end-to-end style testing framework for SB for things like this 😆

[zefhemel]

First of all: thanks so much for doing this work. The PR is pretty large and it's pretty hard to review, so have a look at the few things I mentioned and at the same time I'll do a local checkout and play with it to see if I can make it break.

[zefhemel]

Issue:

When auto completing an image URL with a space in the name, it expands to a syntax with <, e.g.

![bla](</zefzefzef 3/americans-europeans-2.jpeg>)

which then breaks in the live preview (it shows the image as a broken link). I think this may just not be valid markdown at all, and may have to resort to a URL encoded URL?

Update: funny thing is that with links to wiki pages this does work:

[efef](<test attachment page>)

clicking that link works fine.

[zefhemel]

Overall, once again: thanks for making this effort. It's looking really good, and we now will have code complete for attachments, which I'm pretty sure was missing until now. At some point SB may actually become decent for stuff that involves not primarily text files 😆

[onespaceman]

I think that covers those issues. let me know if anything else needs changing

[zefhemel]

This looks almost good to go, just found one issue:

You auto complete a page with [[something it now completes attachment files too, not just pages. Now the question is actually: while a regression, is this perhaps desired? That way you can link to attachments without using []() style links 🤔 Did you do this on purpose?

[zefhemel]

Ok, it is inconsistent with the ![[image.png]] syntax that is also supported now (I even forgot we have this, or did you add it here?) at least when completing ![[image.png I'd expect only attachments (perhaps even just images) to be completed, not pages as well.

To be honest, I'd prefer to keep using [[page]] just for pages, and ![[image.png]] for attached images only (until we make that syntax also support pages and effectively be transclosures: #533).

[zefhemel]

Ok I think you indeed added the ![[image.png]] in this PR as well correct? That's cool, but then I found an additional bug:

• When you have a page in a folder and drag & drop an image into, it translates this to ![[image.png]] which breaks, because it assumes an absolute path (so should include the current folder), so the folder name should be prepended to it. This is different than the previous behavior of using the ![]() syntax, where paths are relative.

[onespaceman]

That should fix the upload path. I'll fix merge conflicts later.

[onespaceman]

Embeding page links is the next thing I'm working on. So if you don't mind it doing nothing for a little bit, I'll just leave it as is.
I was just going to make it a shortcut for a live template.
Making it an editable transclosure is probably a future pr (idk if I even have the knowledge to do it)

[zefhemel]

Sure, that sounds great. Making the tansclosure editable is not a priority imo. Using it as a live template is also what I'd go for.

[onespaceman]

#833 is unfortunately a major pain for refactoring and for autofilling links.
Do you mind if I rewrite it to use ![alt text | resolution](filename.png) format suggested in #493 . It would also match the format for wikilinks ![[image.png|resolution]]

[onespaceman]

pinging @fflorent for input as well

[zefhemel]

Personally I have no strong preference. I don't know to what extent this is any way standardized. Incompatibility with other tools (maybe GitHub not sure of it even supports it) would be by only concern.

[onespaceman]

It's not standardized at all. Obsidian uses the format I suggested, most of the rest, including github use html embeds.
<img src="image.png" width="200" height="100">

This way is probably better because links will still work as normal

[zefhemel]

I'm perfectly fine with changing it then

[onespaceman]

there. should be ready

[zefhemel]

Ok this looks good and seems to work as expected

Two questions remain:

• Auto completing ![[ shows also pages, is this in preparation for the "embedding" that you'll implement later? If so, then maybe it's ok to leave it like this for a bit
• Auto completing [[ also auto completes attachments, is this intended? I can see arguments for it

[onespaceman]

yes to both.
For 2, I'm thinking that wikilinks = local files

[zefhemel]

Alright. Let's do this!

[zefhemel]

Thanks again. This is awesome stuff.