maforget / ComicRackCE

A Community Edition for the legendary Comic Book Manager ComicRack. ComicRack is back from the dead.
GNU General Public License v2.0
295 stars 32 forks source link

When a comic is imported with a trailing space in the series name of the comic info file, filtering doesn't work #103

Closed RogueBurger closed 1 month ago

RogueBurger commented 1 month ago

Describe the bug If a comic has a ComicInfo.xml file with a trailing space in the Series field (e.g. <Series>Foo Bar </Series>), the comic does not filter correctly in the ComicRack UI after import.

Exact Steps to Reproduce Steps to reproduce the behavior:

  1. Import a comic with a trailing space in the Series field of the ComicInfo.xml file. For example:
    <?xml version='1.0' encoding='utf-8'?>
    <ComicInfo xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
    <Series>Space Ghost </Series>
    <Issue>4</Issue>
    <Month>8</Month>
    <Year>2024</Year>
    </ComicInfo>
  2. Create a list and add just that comic to it (not necessary to reproduce the bug, just makes it easier to notice)
  3. Set one of the top level filters to series, and select the single available series to filter on
  4. You should see a completely empty results screen.

Screenshots Screenshot 2024-08-19 114054 Screenshot 2024-08-19 114114

Version/Commit (check the about page, next to the version, for the string between brackets):

Additional context Editing the Series name to remove the trailing space, either in the ComicInfo file itself or via ComicRack does fix the issue.

maforget commented 1 month ago

The list of value shown have the spaces trimmed, so when you select it it doesn't match since you selected "Space Ghost" and only "Space Ghost " exists. And both aren't the same. The reason you have to go to import a book, is only because the Info dialog will automatically trim spaces. But the same could be reproduced via a plugin like Data Manager for example or external programs.

What are you expecting has a resolution?

  1. Trimming entries automatically when checking the ComicInfo.xml:
    • The value was set with a space at the end, it's the data that is wrong.
  2. Show all the possible values in the filtering box without trimming:
    • That entry that is shown, was trimmed, so we show all the possible values.
  3. Selecting the entry would ignore the spaces after it:
    • This is the most likely solution, except that this filtering is just a smart list. Changing this would mean altering the way lists match the value, by always trimming the value that comes from the book. And although spaces like that aren't supposed to happen normally, they do happen like you can attest. I am reticent to change the way smart list work. I just don't know how someone weird setup will stop working because of this.

In the end it's a data error, and I can see the problem. You have 1 entry and you select it it should show it. So the solution that has the least impact would be point 2. It would add multiple times the same entry if for example only 1 entry as extra spaces.

N.B. A quick fix is to run a data manager rule to trim the spaces.

RogueBurger commented 1 month ago

I noticed that if you go to edit the Series name in ComicRack, and you put a trailing space in there, the system automatically trims it when you save. So because CR already doesn't let you put trailing spaces in the series, I feel like option number 1 best fits that by just nipping it in the bud on import.

Ultimately, though, I think any of those three solutions would work just fine. I'm concerned way less about the data itself, and more with the confusion caused by clicking a series filter and having nothing show up.

maforget commented 1 month ago

It seems logical to be number 1, but it isn't that easy to do. There are no real built-in way for that.

This is just the code that is used to read the ComicInfo.xml.

return XmlUtility.GetSerializer<ComicInfo>().Deserialize(inStream) as ComicInfo;

Changing that we would need to go through all the books and all the values to trim them all. This would be expensive. And it would be possible that the space was added by a plugin, then you need to do the same for exporting the file. Then maybe there are values that are already like that in someone DB, then I need to do it for the DB also.

It's one thing to prevent the data from doing that, but we can't prevent all the cases. And like I stated, it's just data. If the data is wrong we can't help it.

I just had a though of what a simple solution would be. It would not add possible duplicates and still work correctly by changing the smart list used when filtering to a regex instead of an equal. When filtering multiple values field it already does that with a list contains that will ignore spaces. So this would not change how smart list work and would prevent duplicates and still match them.

maforget commented 1 month ago

As #104 shows, the regex option doesn't exists on number fields. I will just show the options in the search browser to be exactly what they are, so if 1 entry has multiple variations of spaces they will all be shown.