middleman-blog icon indicating copy to clipboard operation
middleman-blog copied to clipboard

Format-insensitive tagging-V4

Open matiasgarciaisaia opened this issue 8 years ago • 14 comments

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 avatar Aug 31 '16 18:08 matiasgarciaisaia

@matiasgarciaisaia Can you resubmit this please - as Travis should now be fixed in master.

iwarner avatar Mar 24 '17 15:03 iwarner

I am looking into this more today

Trouble I am finding is

  1. 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

iwarner avatar May 17 '17 16:05 iwarner

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.

matiasgarciaisaia avatar May 17 '17 16:05 matiasgarciaisaia

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.

iwarner avatar May 17 '17 16:05 iwarner

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 👍

matiasgarciaisaia avatar May 17 '17 17:05 matiasgarciaisaia

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.

iwarner avatar May 17 '17 17:05 iwarner

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.

markets avatar Feb 04 '24 18:02 markets

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).

matiasgarciaisaia avatar Feb 05 '24 21:02 matiasgarciaisaia

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?

markets avatar Feb 09 '24 18:02 markets

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.

matiasgarciaisaia avatar Feb 09 '24 18:02 matiasgarciaisaia

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!

markets avatar Feb 09 '24 23:02 markets

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.

github-actions[bot] avatar May 10 '24 02:05 github-actions[bot]

@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.

matiasgarciaisaia avatar May 10 '24 04:05 matiasgarciaisaia

Thanks @matiasgarciaisaia! We have plenty of examples under the fixtures folder. Could you please add a test case and update from master 🙏🏼?

markets avatar May 10 '24 11:05 markets