middleman-blog
middleman-blog copied to clipboard
Format-insensitive tagging-V4
Cluster articles by "URL safe" version of tags, so we don't miss articles in listings.
Fix for master
branch.
Fixes middleman/middleman-blog#313
@matiasgarciaisaia Can you resubmit this please - as Travis should now be fixed in master.
I am looking into this more today
Trouble I am finding is
- Its opinionated in terms that it will downcase and merge all the tags on the UI through the usage of safe parametise. This may not be what users want in terms of look.
What we need to do is create some proxy that identifies similar tags even though case may be different.
Also finding though that Safe parametise is not creating URL safe links - using ? in tags for instance is preserved in the link
The specs are failing - and that seems to be OK. We should fix the tests to reflect the change of behaviour - I haven't paid much attention to see what changed on the tests. If you want to help, please be my guess :)
Downcasing the tags is, in fact, kind of opinionated. But I don't see a tag system in which you don't want to have this opinions. You could argue that you shouldn't write the tag with casing if you want it to be case-insensitive, but this is a response to a bug that was happening - the generated tag pages are case-insensitive, so the opinion is already there.
If the question mark is preserved on the link - isn't that a bug in safe_parameterize
?
The original issue is that middleman builds a path using the tag name. So the same set of opinions that we use to build that paths - that same set we should use for the tags.
Totally understand the fixes do solve the rather large bug.. but bear with me
With the changes we are taking tags like
London Paladium and transforming to london-paladium on the UI
I can see stack overflow dictate this UI look for their tags - do we want to do it for all the blog users?
If you can add and fix your failing tests that would be great and resubmit - I will look at options also.
Oh, sorry - if that's happening, then it is true that we're being sub-optimal.
This line tries to deal with the UI - don't show london-paladium
but London Paladium
. Tries, because it gets the first occurrence of the tag as its "label".
We could add some config to map tags with their labels, maybe, instead of picking some version. And maybe that's why the specs are broken - it shows foo-x
instead of fooX
, right?
So the PR may need to be fixed, yeah 👍
Exactly. Its a pain to solve this without being 'opinionated' :)
We need to give the user the option really of being loose or tight in the representation of tags and the articles associated to it.
Right now we could reverse the thinking and when collecting articles by tag look for 'similar' tags in the stack and display the article on match.
Cause right now the main issue is that 'foo' and 'Foo' on different articles link to the same place but only show the foo article. We want it to show both.
Should right tests for this action first, really as it will fail.
Hello @matiasgarciaisaia @iwarner 👋🏼 I'm helping to maintain this project now, could you please summarize if there is any missing action here 🤔? I'm still exploring the code, issues, ... but I agree with the fact that if this is a breaking change, we'll probably need some flag to be able to configure this behavior.
Hi @markets. Glad to know you're going to be helping around here.
I understand this may be a breaking change on the UI side (showing the label in a different way than intended).
But the real issue for me is that there's an underlying bug here that's preventing from showing tags.
The low-hanging fruit would be to detect the naming collision and throwing an exception? In our case, we had articles tagged crystal
and others tagged Crystal
- so some of them wouldn't show up in the tag listing.
Then we can think how to solve the opinionated part of the issue, which may have more nuances than the ones I'm caring about (thinking about whitespace, as @iwarner is).
Thanks for the update @matiasgarciaisaia!
I think I now understand properly the "issue" and I have mixed feelings. We can "force" things, like using this parameterize or any other manipulation, but this is a potential breaking change.
In the current situation, it's just a matter of perseverance of the bloggers maintaining the site. They need to be consistent about how they use the tags, but they have full freedom. Maybe a warning or exception for name collisions is a good compromise?
I think the current approach is broken: I was missing data in production for a long time, unknowingly.
Raising an error would be the absoulte minimum that should be acceptable, I think.
ok @matiasgarciaisaia! Let's raise an error to warn the users falling into this situation. Could you please update your branch implementing this idea? I think it will be nice to have some tests too for this new behavior.
Thanks!
This issue is stale because it has been open for more than 90 days with no activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.
@markets I took a blind shot at this, since I don't have a middleman v4 site in which to test. If you can add some features/specs, that'd be great to eventually merge this.
Thanks @matiasgarciaisaia! We have plenty of examples under the fixtures folder. Could you please add a test case and update from master 🙏🏼?