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

Make wikilinks more flexible (v2) #103

Closed
wants to merge 6 commits into from
Closed

Conversation

digiguru
Copy link
Contributor

Since the foam-core commit it's allowed me to rewrite the work in #93 to work with the new style. I thought I'd start afresh and try to make my classes a little more reusuable.

To start this isn't integrated in the rest of the program, but I can get all the units working here until I'm ready.

@jevakallio
Copy link
Collaborator

@digiguru amazing! Sharing some conversations we had on the Foam Discord with @kortina (Markdown Notes maintainer), here's the current and near-future goal state for Foam:

  1. We want to support [[Title Case]] and [[file-case]] links.
  2. We would like to autocomplete to either, depending on user preference
    2.1. If autocompleting to file-case, ideally the completion should not have extension (.md)
  3. We are considering supporting additional Mediawiki Links features like
    3.1 [[...|Description]] pipes and
    3.2 [[...#anchor]] deep links to sections
  4. We are also evaluating some non-standard features like autocompleting based on actual Titles in files rather than file names, but we haven't decided what the actual link format should then be. Perhaps: [[File Name|Title From File]]?

Markdown Notes already supports 1, is considering supporting 2 and 3.1.

@digiguru the work you're doing seems already pretty well aligned with these, but FYI.

@digiguru
Copy link
Contributor Author

@jevakallio I really appreciate the way you communicate. I'm sure the energy in this project is in now small part feeding off your passion.

I was thinking about MediaLinks....

3.1 [[...|Description]] pipes

This is an awesome feature for creating new files, but I question the value after the file is linked. It seems like a title that could easily go out of date. I think if foam can always "own" the right hand part of the link and keep it up to date, then I can see the value.

@jevakallio
Copy link
Collaborator

This is an awesome feature for creating new files, but I question the value after the file is linked. It seems like a title that could easily go out of date. I think if foam can always "own" the right hand part of the link and keep it up to date, then I can see the value.

I think the idea there would be precisely the ability to override the title on a case by case basis to make it flow better in a sentence structure. It would be a trade-off for the workspace maintainer to choose. Perhaps we shouldn't automatically complete to it.

Foam will have a concept of a linter. Perhaps the linter will warn about out of sync titles.

I really appreciate the way you communicate

Thanks! Just trying to be a good human!

@garyng
Copy link

garyng commented Jul 13, 2020

3.2 [[...#anchor]] deep links to sections

Is it possible for deep-linking to individual paragraphs/list items, like what Dynalist does?

@jevakallio
Copy link
Collaborator

@garyng how does Dynalist do it? Does the target parapraph/list item have to have any id/annotation on it. or can you just link to plan text paragraph?

Because Foam operates on markdown and paragraphs don't have id's, for us to do it we need to decorate the linked-to paragraph with some sort of annotation.

One way would be to use the fact that HTML is valid markdown, and do like Mediawiki links do:

https://www.mediawiki.org/wiki/Help:Links (search for "Setting an anchor")

They required "span" tag, but we could also just use custom html tags and do .

@garyng would this work for you?

@garyng
Copy link

garyng commented Jul 13, 2020

@jevakallio In Dynalist, you can link to any item/paragraph:
2020-07-13_21-06-58

Giving paragraph an id with span tag is a good idea! Is it possible for Foam to autogenerate them?

EDIT:
I think autogenerating IDs for each paragraphs and auto-completing them would be challenging right?

@jojanaho
Copy link
Contributor

Maybe consider something like this for the anchor linking: https://jsfiddle.net/9ukc8dy6/
Basically it converts ## Next section to <h2 id="next-section">Next section</h2>

This could be linked against (with the assistance of auto complete) using [[some-page#next-section]], or in case titles in links are accepted, [[Some Page#Next Section]] (or even [[Some Page#Next Section|My Alt Text]]). If user changes the title (and thus the anchor), we could show invalid links in diagnostics (linting) and maybe even provide quick fix for the error by showing a list of valid headings in the note so that user can pick the new target.

Edit-time in VS Code, we could do the proper scrolling on ctrl-click navigation. The support in the generated HTML depends on the generator. If it doesn't support these "auto-id-anchors", we could then generate anchorless output for its consumption.

The library used in the example is https://www.npmjs.com/package/markdown-it-anchor

@jojanaho
Copy link
Contributor

After educating myself on GFM (e.g. https://gist.github.com/asabaylus/3071099), I see that the title to kebab-case references are well supported as is. Anyway, here's some more thinking on the same topic: #105 (review)

@fmagin
Copy link

fmagin commented Jul 20, 2020

Just found this PR after starting to use Foam today.
I used references like [[Type Theory]] intuitively which worked in the sense that I could click on them and get a new note with the appropriate title, but they didn't show up in the Markdown Links Graph. Which seems logically because the link doesn't actually match any file, so that addon doesn't recognize it as a link. One idea to work around this would be to add a proper link to the generated references.

In my current hacked together setup( commit c11bb52 rebased on 00d1bd3), the references look like:

image

So no entry is in a format that is recognized by the Markdown Links Graph. This doesn't work with just the commit c11bb52 so I didn't expect this to work at all, and this is more of a Proof of Concept.

One workaround is now to wrap the filename in double brackets, so it gets picked up:

image

Now the graph renders properly. The issue is that the part in the single brackets now doesn't match the actual link used anymore for some reason. I am not sure what causes this, the only change I did was add the extra double brackets around ${definition.url}, i.e:

  let text = `[${definition.label}]: [[${definition.url}]]`;
  if (definition.title) {
    text = `${text} "${definition.title}"`;
  }

I might be misunderstanding various things though, like the exact purpose of the autogenerated references and if my workaround horribly breaks this. So all in all I just want to make sure it is brought up here, because this PR is required to make it work in the first place (by adding the edges to the foam internal graph, which is used to populate the references). This feature seems(in my fairly uninformed opinion) a lot simpler than the section linking and complicated links in general, so it might be beneficial to support the above use case soon (because it is something at least I did intuitively and was confused why it only partially worked), while the discussion around the complex link formats is still ongoing.

All in all, I really like this project so far, so if something specific is needed to support this I am happy to contribute, in case it isn't already covered in the scope of this PR.

@digiguru
Copy link
Contributor Author

Hi @fmagin - This PR is in draft and only has test cases so I don;t expect anything to behave differently in Foam (only on the test runner).

However, if you want changes in the graph tool, then you want to contribute to is Markdown Links - specifically the following PR:

tchayen/markdown-links#21

What it is trying to do is adopt another extensions linking style - vscode-markdown-notes. What the teams are trying to figure out is how to be consistent with slugifying, but also flexible to user needs as @wgslr points out.

Generally, I believe it's hard to maintain consistent behaviour between separate extensions, especially in such loosely defined matters like "similar titleness". It would be best to have some shared spec, or a library, defining this stuff. But that's getting ourselves into a xkcd#927 situation, especially since all of these extensions can and probably should support use cases separate from Foam.

@digiguru
Copy link
Contributor Author

I should also state, the fact that the graph works with the change to markdown-provider.ts that you describe that seems to fix the graph is likely to break how links work in Github Pages and or Github Preview. Have a look at this document created by @jojanaho & @jevakallio to discover why those links are the way they are....

https://foambubble.github.io/foam/link-reference-definition-improvements

Have you tried modifying that and seeing the effect on GitHub Pages and Github Preview?

@fmagin
Copy link

fmagin commented Jul 21, 2020

This PR (specifically https:/foambubble/foam/pull/103/files#diff-0b86ab558ba8f9a796ed039776345e2f) does change Foam itself and if combined with some other commit (possibly dc1c237 but I haven't tracked this down. It is definitely one that wasn't in your branch yet yesterday) it changes the behavior in VS Code. Basically your change allows the links to be detected and added to the foam internal graph, which is used in the VS Code extension to generate the references. If those references are now a standardized format, they will be picked up by the displayed Markdown graph.

My hack is definitely absolutely broken for every usecase but mine currently. That was the only thing I was optimizing for last night. For example the reference generation also picks up itself, so a link that is only in the reference section will be found and then added to the reference section when regenerated. Despite it not being used in the actual document itself.

Edit: Removed the discussion around the linked markdown-links issue, because I am sleep deprived and didn't actually understand it properly.

@digiguru
Copy link
Contributor Author

@fmagin - can you create a separate PR for this? I would name it something like Fix markdown links graph that contain whitespace and we can aid manually testing the edge cases, even better write unit tests that prove or disprove them.

It could be that the groundwork you've done can be applied to markdown links - I'm sure we could convince @tchayen if we can recreate it there.

@fmagin
Copy link

fmagin commented Jul 22, 2020

I actually talked to @tchayen about the link issues, and it seems like the only thing that would be needed on foam's side is to properly detect all kind of links foam wants to support and then just add the proper link reference definitions. See tchayen/markdown-links#28 (comment). Markdown-links then just needs to support the reference definition without the .md at the end. I am not sure what the long term plan for that feature is, but it allows both projects to stay generic and support many other plugins.

There are still various cases that aren't handled by foam and whatever creates the new note itself, but those seem like something that could be easily fixed and tested. For example accepting and replacing ' or : in links and replacing them with - in the generated references. I can look into that when I have the time.

To clarify: I didn't do any "groundwork" in that sense so far. I just rebased your commits on some commit from the master branch and things suddenly worked. The only true change I have right now is adding .md to the end references, so markdown-links just works. My code is at https:/fmagin/foam/commits/graphing if you are interested.

@digiguru
Copy link
Contributor Author

Thanks for looking into this @fmagin . I'll see what I can do for you, but can't promise delivery any time soon.

I can see from the regular expression that the case with colons an quotes should be replaced with the slug as you desire...

.replace(/[!"\#$%&'()*+,\-./:;<=>?@\[\\\]^_‘{|}~\s]+/gi, slug)

But like I suggested before it's not all linked together (yet)

@jevakallio
Copy link
Collaborator

Closing this in favour of foambubble/rfcs#3

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.

5 participants