D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[Christoph Cullmann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R246:454d4d8be9dc: DictionaryComboBox: Keep user preferred 
dictionaries on top (authored by loh.tar, committed by cullmann).

REPOSITORY
  R246 Sonnet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17030?vs=47194=47202

REVISION DETAIL
  https://phabricator.kde.org/D17030

AFFECTED FILES
  src/core/settings.cpp
  src/core/settings_p.h
  src/core/speller.cpp
  src/core/speller.h
  src/ui/configui.ui
  src/ui/configwidget.cpp
  src/ui/dictionarycombobox.cpp

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[Christoph Cullmann]

cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.


  The intermediate diff looks strange, but the final diff is IMHO ok. Will 
apply that, thanks.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a comment.


  Um, I'm bit confused when I now look at the diff 1/2 here???

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar updated this revision to Diff 47194.
loh.tar added a comment.


  - Add @since hint to speller.h
  - Add some blank lines to speller.h to be looking a little bit nicer

REPOSITORY
  R246 Sonnet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17030?vs=45842=47194

REVISION DETAIL
  https://phabricator.kde.org/D17030

AFFECTED FILES
  src/core/settings.cpp
  src/core/settings_p.h
  src/core/speller.cpp
  src/core/speller.h
  src/ui/configui.ui
  src/ui/configwidget.cpp
  src/ui/dictionarycombobox.cpp

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[Christoph Cullmann]

cullmann added a comment.


  In D17030#373994 , @loh.tar wrote:
  
  > In D17030#373912 , @cullmann 
wrote:
  >
  > > Ah, one thing: Could you add some @since 5.xxx to the new function in 
speller.h?
  >
  >
  > Only in speller? In settings_p.h are also two new, but there was no docu, 
so I also wrote nothing, but could
  
  
  I think only speller.h is installed as public API, yes.
  
  > 
  > 
  >> I assume 5.54 would be the right one.
  > 
  > In CMake it's currently `KF5_VERSION 5.53.0` so 5.54 is the right one?
  
  I think so.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a comment.


  In D17030#373912 , @cullmann wrote:
  
  > Ah, one thing: Could you add some @since 5.xxx to the new function in 
speller.h?
  
  
  Only in speller? In settings_p.h are also two new, but there was no docu, so 
I also wrote nothing, but could
  
  > I assume 5.54 would be the right one.
  
  In CMake it's currently `KF5_VERSION 5.53.0` so 5.54 is the right one?

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[Christoph Cullmann]

cullmann requested changes to this revision.
cullmann added a comment.
This revision now requires changes to proceed.


  Ah, one thing: Could you add some @since 5.xxx to the new function in 
speller.h? I assume 5.54 would be the right one.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[Christoph Cullmann]

cullmann added a comment.


  I think this makes sense, it improves the usability of the dictionary 
selection.
  I see no API issues, should be BC and docs are there, too.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar edited the summary of this revision.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a comment.


  #Frameworks  Ping! (?)

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a comment.


  Bug302689 - Impossible to delete unwanted dictionaries
  https://bugs.kde.org/show_bug.cgi?id=302689
  
  Does not quite fit, but relieves the symptoms. Enough to close that?

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Makes sense to me, visually speaking!

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar edited the test plan for this revision.
loh.tar added a reviewer: VDG.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a reviewer: Frameworks.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a comment.


  The added stuff at the ConfigWidget looks to me not perfect but OK. 
Suggestions are welcome. Some thoughts/questions
  
  - Most UI member are named m_foo but some without the m_ prefix. I can't 
recognize a rule why and chose for my new stuff a name without the prefix due 
to the way how they are used later, by ui-dot (ui.foo). So my offer is to 
rename all uniform, let me know which you prefer
  - The DictionaryComboBox and the new QListWidget (languageList) is somehow 
reduntant. Would be nice to include the "default language information" into the 
languageList. But how? The only idea I had was to add an icon to the "default 
language" by a double click.
  - Changes at the selected languages appear not direct in the 
DictionaryComboBox, only at next run. The same rules to the list itself. It 
would be a little unsettled to reorder the list by each change.
  - Not investigated if in Settings::setPreferredLanguages is calling 
d->loader->changed() is needed. Guess it is, but why not in every setFoo 
function?

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

To: loh.tar
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar created this revision.
Herald added a project: Frameworks.
loh.tar requested review of this revision.

REVISION SUMMARY
  ...to ease the switch between dictionaries you usually need

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D17030

AFFECTED FILES
  src/core/settings.cpp
  src/core/settings_p.h
  src/core/speller.cpp
  src/core/speller.h
  src/ui/configui.ui
  src/ui/configwidget.cpp
  src/ui/dictionarycombobox.cpp

To: loh.tar
Cc: kde-frameworks-devel, michaelh, ngraham, bruns