search

QNapi / qnapi

Qt-based, multi-engine, multi-platform subtitle downloader
http://qnapi.github.io
291 stars 39 forks source link

Consistently use the 'remove_words_enabled' setting #135

Closed rplociennik closed 6 years ago

rplociennik commented 6 years ago

Additionally, rename associated 'cbRemoveLines' to 'cbRemoveWords' in frmOptions to match the PostProcessingConfig setting.

rplociennik commented 6 years ago

Well, apparently there was a problem with 3rd party PPA with Qt 5.9 while performing one of Travis' builds...

krzemin commented 6 years ago

Can you please update installing right version of the PPA in travis build script? I guess replacing ppa:beineri/opt-qt59-trusty with ppa:beineri/opt-qt593-trusty should make it pass.

rplociennik commented 6 years ago

Right, there we go.

rplociennik commented 6 years ago

Alright, I hope I haven't overdone it this time.

The changes have been split into smaller commits and described to better reflect the idea behind them.

I've decided to preserve the old remove_lines and simply make sure that the setting gets properly saved and loaded which was pretty much the main intention all along.

Secondly, since the meaning of remove_lines and remove_words residing in a single config file seems unclear, I'm proposing changing the latter to remove_lines_words.

All renames are followed up by refactors across other classes in order to maintain consistency.

krzemin commented 6 years ago

I think your initial naming proposal was quite good. Anyway - the most important is to have backwards compatibility kept and decent naming consistency.

I'm happy to merge this PR. Thank you for valuable piece of work 👏

rplociennik commented 6 years ago

Thank you for accepting these changes and for the kind words.

I only hope I could have described them more precisely to begin with. ;)