Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leading Comma for Genres #456

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Katazui
Copy link

@Katazui Katazui commented Mar 10, 2025

Issue Fixed

The PR addresses a bug in note field processing where:

  1. Empty note fields were incorrectly processed - split(',') on an empty string created an array with one empty string element
  2. This empty string was being added to metadata as a legitimate tag value

Fixes #447 - Prevents empty attributes from appearing in tags panel and being saved

Changes Made

- let note = this.tags[field]??[];
+ let noteValues = this.tags[field] ?? [];
  
- return note.join(',');
+ return this.processNoteString(noteValues.join(',')).join(',');

- for (let value of this.note.split(',')) {
-   if (value.trim())
-     out.push({value, type: 'note', index: I});
+ for (let value of this.processNoteString(this.note)) {
+   out.push({value: value, type: 'note', index: I});

New Helper Methods Added

// Process note string
processNoteString(noteStr: string | undefined): string[] {
    if (!noteStr) return [];
    return noteStr.split(',')
        .map(n => n.trim())
        .filter(n => n.length > 0);
}

// Similar method for genres
processGenreArray(genres: string[]): string[] {
    return genres
        .map(g => g.trim())
        .filter(g => g.length > 0);
}

These changes ensure empty values are properly filtered out when processing both note and genre fields, preventing unwanted empty tags.

Copy link
Owner

@Marekkon5 Marekkon5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, thank you very much for the PR, tested locally and it is working. I only noticed that there is one unused function, would you please remove it or use it?

Thank you.

@Katazui Katazui requested a review from Marekkon5 March 11, 2025 13:12
Copy link
Owner

@Marekkon5 Marekkon5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unused function is still there, is this intentional?

@Katazui
Copy link
Author

Katazui commented Mar 12, 2025

Honestly, that was my mistake, working late nights!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants