search

TASVideos / tasvideos

The code for the live TASVideos website
https://tasvideos.org/
GNU General Public License v3.0
62 stars 29 forks source link

Wiki links with spaces are rendered without spaces #1858

Closed Randomno closed 4 months ago

Randomno commented 4 months ago

For example on https://tasvideos.org/RecentChanges, the page has a link to [new movies], but this is rendered as Newmovies. It was previously rendered as new movies until 1c8034d.

nattthebear commented 4 months ago

[new movies] gets normalized to a url of /Newmovies, which is correct and has not changed. Formerly, we passed the original unnormalized text to use as the displaytext argument to __wikiLink. Since 1c8034d679136dec196126ffcfa1fbac5f0c07f6, we don't do that because we need to distinguish cases where the user didn't explicitly provide display text and replace those with canonical object titles. So [123M] gets the display text "NES Guerrilla War by lithven in 14:39.18", but [123M|123M] gets the display text "123M" and the tooltip text "NES Guerrilla War by lithven in 14:39.18".

I think to resolve this case we'd need to distinguish between "strong" display text, where the user explicitly provided something after the |, and "weak" display text, where the display text was inferred from the link, but due to the normalization process, still might have some textual difference from the link.

The effective behavior is the result of a combination of logic interacting in both https://github.com/TASVideos/tasvideos/blob/b09dee4810d727256e5b214f8aaa7f03b145430c/TASVideos.WikiEngine/MakeBracketed.cs and https://github.com/TASVideos/tasvideos/blob/ac84694cde36974a5e3e3834b8f6c69716d39e6a/TASVideos/WikiModules/WikiLink.cshtml.cs. So what we really need are some automated tests that test the entire wiki process end to end, starting at wiki markup and ending with the final HTML output, including any Razor module results. I can easily test the wiki by itself, but I don't know how to make meaningful offline tests that boot up the entire MVC+Razor system.

adelikat commented 4 months ago

[recent changes] is effectively syntactical sugar for [RecentChanges|recent changes, maybe this isn't robust enough for syntactical sugar and the effort isn't worth the value? We could just drop that support in a way that causes a broken link such that we can go through and fix those instances

nattthebear commented 4 months ago

[recent changes] is effectively syntactical sugar for [RecentChanges|recent changes]

I agree with the sentiment, it was just accidentally lost while also trying to make [123M] syntactical sugar for [123M|NES Guerrilla War by lithven in 14:39.18] while also making [123M|Gorilla War] syntactical sugar for [123M|Gorilla War|title=NES Guerrilla War by lithven in 14:39.18].

I don't object to any specific behavior of the bracket syntax, but there are quite a few of them and it's proven difficult to keep track of all of the expectations. If we can't create tests of the sort I described in my previous post, we might consider simplifying instead.

Randomno commented 4 months ago

I'd prefer to keep [recent changes], though it's not essential. Removing it wouldn't affect external links to the site.