-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: Add a simplify_title
option to the preferences
#56
Conversation
Tweak wording of setting Tweak unabridged REGEX Remove unintentionally added code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Check the comments I left - hopefully they are clear to understand.
Also, if you cannot think of a concise/short way to describe the feature as a preference label, it may need a README addition.
Ok I made the changes requested, let me know what you think! And as for the README, I can add mention about this setting if we can't think of another alternative, but if we wanted to do that, where do you think it should go? |
Looks good now! I changed the merge to go into develop, since I follow a develop->main workflow. This also gives people the chance to test develop changes before it becomes a release for all. |
Awesome, sounds good to me! Thanks for taking the time to review this! |
* feat: Add a `simplify_title` option to the preferences (#56) * feat: Add a `simplify_title` option to the preferences Co-authored-by: David Dembeck <71412966+djdembeck@users.noreply.github.com> * chore: 🔖 bump version * chore(release): 0.4.0 Co-authored-by: Chris Sandvik <9214195+csandman@users.noreply.github.com>
I've recently been trying to switch to using this Agent over the one from seanap, but it has a few things I want to change before I'd be willing to, as those things go against what I'm looking for data wise (one of those things being original publish date). Of course, I don't expect all of my changes to get merged into the main Agent as they're specific to my preferences, but I figured I'd submit what I can if it doesn't conflict with what you have already. Besides that I'll probably just maintain my own fork with the rest of my changes that are specific to my preferences.
I know in
v0.2.1
you added the subtitle to each book's title. As you mention in #10 you did so because the relevant part of the title is sometimes only in the subtitle. As for whether or not this is true, I believe that Audible has been switching the titles on a lot of their books to include the meat of the title in the main title section for the book. In my opinion, this leads to some very repetitive and overly long title fields when you also include the subtitle. Even for the example you give, the subtitle is now just a piece of the main title, and using the subtitle leads you to haveDeath Troopers: Star Wars Legends: Death Troopers
.I thought that the best way to handle this for people like me who want cleaner/more minimal looking titles would be to add an option for it, so as not to break the existing behavior. This option would prevent the agent from appending the subtitle, and perform further simplification transformation on it to make it look as clean as possible.
The additional steps it takes are as follows:
, Book N
, indicating a part in a series (i.e.Book 1
orBook One
). I don't think this part of the title looks appealing as its not part of the book's actual title, just something they use to further identify it.I'll use a Harry Potter book as an example: https://www.audible.com/pd/Harry-Potter-and-the-Order-of-the-Phoenix-Book-5-Audiobook/B017V4NMX4
The title for this book is
and this setting would change it to
I have found that Audible does this with decent frequency, and I use this transformation in my own app I shared with you.
(Unabridged)
or(Abridged)
text from the end of the title. I'm not sure if you're already doing this at any point before this, so if this is unnecessary let me know. I believe you're doing it before running a search and on the search results in order to match better, but I'm not sure if you're keeping it that way for the final title. Regardless, I think this is a good change to keep the titles looking minimal. Like I mention in the first transformation, this is not part of the book's title, just a way of describing it.Let me know if this change works for you! Overall I think its a nice option to have for those like me who want the title to look as clean and close to the original as possible. Also definitely let me know if you want me to make any changes before it's good to go!
EDIT: Sorry, a couple unrelated code formatting changes slipped into this PR that were done by my editor's formatter, hopefully thats not a problem!