search

metabrainz / picard-plugins

Picard plugins: use 1.0 branch for Picard < 2.0 (python 2/Qt4) and 2.0 branch for Picard >= 2.0 (python 3/Qt5)
https://picard.musicbrainz.org/plugins/
145 stars 94 forks source link

pr fixing issue [abbreviate_artistsort] exception when artist sorted name has no last name part issue#350 #365

Closed sambhavnoobcoder closed 8 months ago

sambhavnoobcoder commented 8 months ago

added check to see if the artist name has a surname or not , and in case the exception occurs , the code recognises the name as a single word sorted name , and then the code generates initials for this single word to serve as an abbreviated representation.The initials are formed by taking the first letter of each part of the single word and appending a dot ('.') after each letter.The code then constructs a new sorted name by combining the original single word (treated as the forename) with its generated initials, separated by the predefined separator.Additionally, the code maintains the logic for handling whitespace and building the new unsorted name in alignment with the adjustments made to the sorted name, ensuring consistency between the sorted and unsorted names.

Sophist-UK commented 8 months ago

Hi @sambhavnoobcoder

In #350 you asked me to review this, however I wrote it so long ago and the code DIFF has such big changes that I am not able to review it without spending a reasonable amount of time on it, and I simply don't have that time available. Sorry.

sambhavnoobcoder commented 8 months ago

okay , so I'll consider it done from my end for the timebeing and move to some other issue for now , and whenever you find the time , just review the pr and if you find it satifsactory, merge it with the main project .

Sophist-UK commented 8 months ago

I am not going to find the time in the foreseeable future - and in any case I am not an admin on Picard Plugins and cannot approve this.

sambhavnoobcoder commented 8 months ago

okay . thanks anyway .

Sophist-UK commented 8 months ago

There was no need to close this. Someone else will review it if you reopen it.

sambhavnoobcoder commented 8 months ago

okay , i'll reopen

phw commented 8 months ago

@sambhavnoobcoder For some reason I can't reopen this and see no changes. But please do submit a new PR. I'll do my best to review this.

I'm not familiar with the code and don't fully understand the functionality, though. I would have hoped Sophist-UK could review this. Do we have some test case in the MB database for which we can test the functionality and the bug?

Sophist-UK commented 8 months ago

@sambhavnoobcoder For some reason you force pushed the original back to this PR so there are no changes and nothing to review.

sambhavnoobcoder commented 8 months ago

ok i'll submit a new pr .

Sophist-UK commented 8 months ago

  1. For the future, force pushes are generally not something you should be using for PRs.

  2. If you are going to submit a new PR, please try to localise changes to make it easier to review - perhaps make changes in several separate commits so that the changes in each commit are more localised and can be individually reviewed.

sambhavnoobcoder commented 8 months ago

ok @Sophist-UK , ill keep that in mind from now on . I have made a new pr at https://github.com/metabrainz/picard-plugins/pull/367 , it would be great if you and @phw had a brief look .