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

added processing of inclusions in link targets #165

Closed
wants to merge 2 commits into from
Closed

added processing of inclusions in link targets #165

wants to merge 2 commits into from

Conversation

xithan
Copy link
Collaborator

@xithan xithan commented Mar 20, 2014

This should fix the problem that inclusions doesn't work on the left side of named links as mentioned in the documentation: http://wagn.org/links (section Notes)
e.g. you can now do things like [[ {{linktarget|raw}} | link text ]]
where the content of the "linktarget" card is something like "http://wagn.org"

e.g. [[ {{linktarget|raw}} | link text ]]
@GerryG
Copy link
Collaborator

GerryG commented Mar 21, 2014

Does this actually accomplish what is needed for this use case? The old version only objectified external links and I suspect there must have been a good reason for that. Does this recursively parse the inclusion? (http://wagn.org in the example)

@GerryG
Copy link
Collaborator

GerryG commented Mar 21, 2014

I'm not sure that this will update references on the left side in the case of a card rename. Make a test that renames linktarget to something else and make sure it gets renamed.

@GerryG
Copy link
Collaborator

GerryG commented Mar 21, 2014

The only mention of left side inclusions on the links card is to warn against them. The problem mentioned in the last comment could be fixed if we can justify the use case. Why do you think you want it? Better yet, you could create an Idea card at wagn.org for this.

@ethn
Copy link
Collaborator

ethn commented Mar 21, 2014

hmm, good questions. it's definitely worth adding some tests. it seems possible that this code will work in simple cases but break in complex cases. If so, then the discussion becomes whether it's better not to support x or to half support x. Usually I prefer non-support, because half-support can leave folks frustrated and confused.

fwiw, my common workaround is to write the link in html (....)

@xithan
Copy link
Collaborator Author

xithan commented Mar 21, 2014

The use case is: If you use a url very often in many cards you want to have it stored separately in case you have to change it one day.

You are right, Gerry, renaming doesn't work with this approach. I've created an Idea card for this: http://wagn.org/inclusions_to_define_link_targets. It would be nice to have a clean solution but for now I can live with the html workaround.

@ethn
Copy link
Collaborator

ethn commented Mar 23, 2014

definitely agree about the need for a cleaner solution. would be nice to have good plans for this before we work on the gui link editor (which may be pretty soon)

@ethn
Copy link
Collaborator

ethn commented Mar 31, 2014

So I'm under the impression that we're tabling this PR for now. (closing). Definitely looking forward to tackling deeper solutions!

@ethn ethn closed this Mar 31, 2014
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.

3 participants