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

Support short links #80

Closed
wants to merge 9 commits into from
Closed

Support short links #80

wants to merge 9 commits into from

Conversation

Nadrieril
Copy link
Contributor

This adds support for short <1234567> links, as discussed in #59. In fact what this does is render any [foo](1234567) link as a zettel link. That can easily be changed if needed.

Some questions: where should I document this ? Do you use a Haskell formatter and if yes which one ?

@srid
Copy link
Owner

srid commented Apr 10, 2020

render any [foo](1234567) link as a zettel link

Can you post a screenshot? What happens to the "foo" in HTML view?

@srid
Copy link
Owner

srid commented Apr 10, 2020

Do you use a Haskell formatter and if yes which one ?

ormolu. See https://github.com/srid/neuron/blob/master/CONTRIBUTING.md

where should I document this ?

./guide zettelkasten.

@Nadrieril
Copy link
Contributor Author

Can you post a screenshot? What happens to the "foo" in HTML view?

[foo](1234567), <1234567> (which is the same as [1234567](1234567)) and [1234567](z://) are all rendered exactly the same. The foo is dropped entirely. I imagined this would only be temporary: I would expect [foo](1234567) to be a link with text "foo".

@Nadrieril
Copy link
Contributor Author

How can I make tests for this new feature ? I would imagine tests that test both the html output of a file and the graph output of a bunch of files, but I don't know how to write that myself.

@srid
Copy link
Owner

srid commented Apr 10, 2020

I think you should ignore the [foo](1234567) case. That can be handled in a different PR with a concrete spec.

This PR should handle the single case of <1234567> (same as [1234567](1234567)), making it behave the same as [1234567](z:).

How can I make tests for this new feature ?

I've been meaning to begin writing tests for link actions, but if you want to take stab, go ahead! Tests use hspec. Maybe add a test/Neuron/Zettelkasten/Link/ActionSpec.hs that tests linkActionFromLink by running it over each individual cases. See IDSpec.hs for reference implementation.

@srid
Copy link
Owner

srid commented Apr 10, 2020

I think you should ignore the [foo](1234567) case.

From the docs in the diff,

You may also annotate the link (ignored by neuron):

- [2008403](z://foo-bar). 	- [foo-bar](2008403).

I like the annotation feature in general. But if we finalize the spec, then we can't expect it to be a link with text "foo-bar" in the future (that would not be backwards compatible).

I suppose the user could specify that the link should be rendered normally using a query like this:

[foo-bar](2008403?linkTheme=simple)

guide/2011504.md Show resolved Hide resolved
@Nadrieril
Copy link
Contributor Author

I like the annotation feature in general.

What's the use for annotations ? Is it to make it easier to know which zettel it corresponds to ? Then I'd say <1234567?comment=foo-bar>, makes a lot more sense semantically than ignoring the link text.
But more generally, for a thing that kinda functions as a wiki, it probably makes sense to sometimes use human-readable ids.

if you want to take stab, go ahead!

I'll let you do it, you know the codebase. I'll happily add tests to it once it's started though.

@srid
Copy link
Owner

srid commented Apr 10, 2020

Yes, annotations are mainly to act as an immediate visual hint when editing a zettel to know what a particular zettel ID refers to at first glance without having to open it.

I prefer something like <1234567#foo-bar> (i.e., the "anchor" is ignored) for annotations. Although anchors could potentially be used to link to a particular section of the zettel. But this can happen in different PR I guess.

But more generally, for a thing that kinda functions as a wiki, it probably makes sense to sometimes use human-readable ids.

The proposal at #70 currently stands at supporting alphanumeric IDs. Perhaps we could add extra characters, like -, to it - so that human-readable ids like "foo-bar.md" is supported?

I'll let you do it, you know the codebase. I'll happily add tests to it once it's started though.

Alright.

srid added a commit that referenced this pull request Apr 10, 2020
@srid
Copy link
Owner

srid commented Apr 10, 2020

Added tests and merged in 0aac0fa

@srid srid closed this Apr 10, 2020
@Nadrieril Nadrieril deleted the short-links branch April 10, 2020 19:44
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