ctolkien / Slugify

Simple Slug / Clean URL generator helper for Microsoft .NET framework / .NET Standard.
MIT License
93 stars 13 forks source link

DeniedCharactersRegex broken in current NuGet build #52

Closed poke closed 1 year ago

poke commented 1 year ago

(tl;dr: You need to release fixes to NuGet)

So, this was a “fun” one. When attempting to use the library from NuGet, DeniedCharactersRegex doesn’t work and the library will always return an empty string, regardless of what you pass as input string.

To reproduce

Simple example, create a new console application and add the dependency:

dotnet new console
dotnet add package Slugify.Core

Edit the Program.cs to contain the following:

using Slugify;

var config = new SlugHelperConfiguration();
config.DeniedCharactersRegex = "[^abc]";

var helper = new SlugHelper(config);
Console.WriteLine($"'{helper.GenerateSlug("abcdef")}'");

Run the project:

PS > dotnet run
''

Analysis

As it turns out, the following code is the problem:

https://github.com/ctolkien/Slugify/blob/e35bf8d4dc55cef5aec678d788348669b4f9df00/src/Slugify.Core/SlugHelper.cs#L49-L51

In the released NuGet version, this appears to be compiled to the following:

sb.Clear();
sb.Append(DeleteCharacters(sb.ToString(), deniedCharactersRegex));

Originally, I thought this was some compiler optimization bug where it would inline the ToString() call here. But looking further, I realized that the released NuGet version is actually a bit older and this is actually the real code there:

https://github.com/ctolkien/Slugify/blob/e9c5e8905c08e8ec78a831915edaa3b89233d209/src/Slugify.Core/SlugHelper.cs#L49-L50

Sooo, this was fixed in #22, and apparently that change never made it onto NuGet (along with some of the other fixes that happened last year). So can you publish a new version to NuGet soon? 😁

ctolkien commented 1 year ago

Related: https://github.com/ctolkien/Slugify/issues/27

poke commented 1 year ago

So you don’t want to do a NuGet release manually and keep all these bugs in until you can automate the process? 😅

ctolkien commented 1 year ago

Hey @poke - finally got around to this... didn't actually mean for it to run... but here we are.. v4 just went out!

ctolkien commented 1 year ago

Spoke to soon, sent this to the local github nuget package repo.. not nuget for real

ctolkien commented 1 year ago

Fixed for reals now.!!!