D18125: KateStatusBar: Add dictionary button

[Christoph Cullmann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:08d1d681ff8b: KateStatusBar: Add dictionary button 
(authored by loh.tar, committed by cullmann).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18125?vs=52307&id=52987

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

AFFECTED FILES
  src/view/katestatusbar.cpp
  src/view/katestatusbar.h
  src/view/kateview.cpp
  src/view/kateview.h

To: loh.tar, #ktexteditor, cullmann
Cc: cullmann, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, domson, michaelh, bruns, demsking, sars

D18125: KateStatusBar: Add dictionary button

[Christoph Cullmann]

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


  Thanks for taking care of the overhead by adding the new internal signal.
  I think this can now go in.
  I still would like to have the button configurable, but that is no reason to 
deny it.

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

To: loh.tar, #ktexteditor, cullmann
Cc: cullmann, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, domson, michaelh, bruns, demsking, sars

D18125: KateStatusBar: Add dictionary button

[loh tar]

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


  - Remove '-w_accents -variant_0' and such from dict-code to keep it small and 
clean
  - Fix to show correct dict when cursor is at end of a range
  - Add signal caretChangedRange to ViewPrivate, much less overhead this way to 
update the info button

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18125?vs=50763&id=52307

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

AFFECTED FILES
  src/view/katestatusbar.cpp
  src/view/katestatusbar.h
  src/view/kateview.cpp
  src/view/kateview.h

To: loh.tar, #ktexteditor, cullmann
Cc: cullmann, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, michaelh, bruns, demsking, sars

D18125: KateStatusBar: Add dictionary button

[loh tar]

loh.tar updated this revision to Diff 50763.
loh.tar set the repository for this revision to R39 KTextEditor.
loh.tar added a comment.


  - Apply nullptr check
  - Add non preferred dict to menu chose by user
  - Reorder actions
  - Remove gone "Spellcheck Selection" action

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18125?vs=49099&id=50763

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

AFFECTED FILES
  src/view/katestatusbar.cpp
  src/view/katestatusbar.h

To: loh.tar, #ktexteditor, cullmann
Cc: cullmann, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, bruns, demsking, sars

D18125: KateStatusBar: Add dictionary button

[loh tar]

loh.tar added a comment.


  > m_dictionaryGroup->checkedAction() might be null.
  
  Oops! Thanks.
  
  > For the naming: Given we show the encodings with their short-names, too, I 
can live with de_DE or stuff like that.
  
  Is it OK to add some cutting like that? Currently is that extra stuff shown
  
  - en-wo_accents -> en
  - en_AU-variant_0 -> en_AU
  
  > For the overhead: I would like to have this optional and disabled per 
default, that avoids all that work in cursorPositionChanged()
  >  Just like we do for word count/...
  
  Hm, would be nice I could do this in an extra patch. At best after my 
"general config interface idea" (where I have nothing tried to do as of now)
  Did you do some perf tests or simiar, is it so much extra work?
  Ideas to reduce the overhead?

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

To: loh.tar, #ktexteditor, cullmann
Cc: cullmann, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, bruns, demsking, sars

D18125: KateStatusBar: Add dictionary button

[Christoph Cullmann]

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


  I played with this patch now ;)
  
  First: to now have KWrite crash on start, one need to modify one line to:
  
  if (!m_dictionaryGroup->checkedAction() || 
(m_dictionaryGroup->checkedAction()->data().toString() != newDict) || 
m_dictionary->text().isEmpty()) {
  
  m_dictionaryGroup->checkedAction() might be null.
  
  For the naming: Given we show the encodings with their short-names, too, I 
can live with de_DE or stuff like that.
  
  For the overhead: I would like to have this optional and disabled per 
default, that avoids all that work in cursorPositionChanged()
  Just like we do for word count/...

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

To: loh.tar, #ktexteditor, cullmann
Cc: cullmann, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, bruns, demsking, sars

D18125: KateStatusBar: Add dictionary button

[loh tar]

loh.tar added a subscriber: cullmann.
loh.tar added a comment.


  @cullmann @dhaumann A short not how to progress would be nice.
  
  - Keep codes as they are
  - Shorten codes as suggested
  - Replace codes by flags/ensign (no idea how)
  - Add (unneeded) label
  - Show (unneeded long) full name
  
  The latter two consume to much space for my taste and give only the very new 
user a benefit, once seen/clicked it's clear.
  
  Here a shot from an attempt to address some issues:
  
  - No "Preferred Dictionaries" header at the separator to not confuse when 
user has a non preferred dict in use (add dict to menu is todo)
  - Change order of entries with the intend to make more often used stuff 
accessible more easy (still experimental)
  - Removed "Spellcheck Selection...". I would like remove that from the main 
menu too and modify the normal "Spelling..." action to behave "smart" as many 
other actions
  
  F6551960: 1547653694.png 

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

To: loh.tar, #ktexteditor
Cc: cullmann, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, bruns, demsking, sars

D18125: KateStatusBar: Add dictionary button

[loh tar]

loh.tar added a comment.


  I'm very much against such a label, and against the full name. 
  "UTF-8" has also no label and that's good. The only stuff where a description 
label is needed are these line/column/word hints. Without a label are you 
almost lost to know which is what.
  
  However, in some later change is planed to add a couple of options to 
customize the bar. So if someone insists on this label it will be then one 
option more. If
  Dominik's suggest is realized that the bar will become "smart to hide 
obsolete stuff when the space is scarce", I guess such a label will go as one 
of the first.
  
  As said, I like to shorten it even more and cut at least these "extra stuff".
  I have only a few dict installed, so any help is appreciated if there are 
other county codes possible than "xx_YY-". I like to modify it to prefer 
"YY" and fall back to "xx"
  
  - "en_GB-ize-w_accents" -> GB
  - "en-variant_0" -> en
  - "en" -> en
  
  
  
qDebug() << Sonnet::Speller().availableLanguages();

"de_AT", "de_BE", "de_CH", "de_DE", "de_LI", "de_LU", "en", "en-variant_0", 
"en-variant_1", "en-variant_2", "en-w_accents", "en-wo_accents", "en_AU", 
"en_AU-variant_0", "en_AU-variant_1", "en_AU-w_accents", 
"en_AU-wo_accents", 
"en_CA", "en_CA-variant_0", "en_CA-variant_1", "en_CA-w_accents", 
"en_CA-wo_accents", "en_GB", "en_GB-ise", "en_GB-ise-w_accents", 
"en_GB-ise-wo_accents", "en_GB-ize", "en_GB-ize-w_accents", 
"en_GB-ize-wo_accents", "en_GB-variant_0", "en_GB-variant_1", 
"en_GB-w_accents", 
"en_GB-wo_accents", "en_PH", "en_US", "en_US-large", "en_US-variant_0", 
"en_US-variant_1", "en_US-w_accents", "en_US-wo_accents", "he"

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

To: loh.tar, #ktexteditor
Cc: ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, 
michaelh, bruns, demsking, cullmann, sars

D18125: KateStatusBar: Add dictionary button

[Nathaniel Graham]

ngraham added a comment.


  If we stay with `en_US`, I'd like to see a label so people know what it is. 
Otherwise, we should use the long name, because then it becomes obvious. So 
here are the two options I would approve of:
  
  - Dictionary: en_US v
  - American English v

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

To: loh.tar, #ktexteditor
Cc: ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, 
michaelh, bruns, demsking, cullmann, sars

D18125: KateStatusBar: Add dictionary button

[loh tar]

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


  - Add missing signals to watch dict changes
  - Ensure button is set on first show
  
  Looks fixed, give it a try.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18125?vs=49078&id=49099

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

AFFECTED FILES
  src/view/katestatusbar.cpp
  src/view/katestatusbar.h

To: loh.tar, #ktexteditor
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D18125: KateStatusBar: Add dictionary button

[loh tar]

loh.tar added a comment.


  > Isn't en_US too cryptic?
  
  Well, yes.
  
  > I suggest to use the just added KLanguageName (see D10446 
). With this, you can get a nice name like 
"English (USA)". Alternatively, you may use 
QLocale("en_US").nativeLanguageName(), but I did not test this, so it may be 
incorrect code.
  
  I have not chose that name to keep it short as possible. I think your suggest 
is used by Sonnet and you see it in the menu. 
  For me is that sufficient. All you need in the bar is a hint that the dict is 
somehow set/changed, or not. For details, click the button.
  How about to cut it even more, just "en" or "US"? Ideas to the flag 
suggestion?
  
  > - What if you have multiple views of the same document, does it still show 
the correct language, even if the language is changed in one view?
  > - What if you have multiple mainwindows (View > New Window), since then you 
have multiple StatusBars. Will that still all correctly work?
  
  Quick tested, behave a little strange. 
  Shows sometimes a dummy text, e.g. after a fresh "New Window", or when you 
have two views and you change in one view the document the first time to the 
new created test file.
  
  Als long as only one dict is in use is no update done. Move you to some text 
with a different dict the button is updated. Changing the dict (on the first 
Window) updates the button too.
  
  Ideas? Which signals are to catch?

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D18125: KateStatusBar: Add dictionary button

[Dominik Haumann]

dhaumann added a comment.


  Isn't en_US too cryptic? I suggest to use the just added KLanguageName (see 
D10446 ). With this, you can get a nice 
name like "English (USA)". Alternatively, you may use 
QLocale("en_US").nativeLanguageName(), but I did not test this, so it may be 
incorrect code.
  
  Besides that, some other questions:
  
  - What if you have multiple views of the same document, does it still show 
the correct language, even if the language is changed in one view?
  - What if you have multiple mainwindows (View > New Window), since then you 
have multiple StatusBars. Will that still all correctly work?

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D18125: KateStatusBar: Add dictionary button

[loh tar]

loh.tar added a comment.


  The shown dictionary is sometimes "wrong"
  
  - Mark a word by double-click and choose a dictionary different from the 
default for that word
  - Click/move into the word -> The shown dictionary fit
  - Double-Click that word again -> The shown dictionary is the default one
  
  Due to the way how the dictionary is reported is it correct. It's always 
right from the cursor position and when the word will selected is the cursor at 
the end of the word. (To talk about it, helps! Add a check for selection and 
ask right from cursor in cursorPositionChanged() may fix that)
  
  The language/dictionary code is sometimes a little long, e.g. 
"en_US-wo_accents".
  
  - How about to cut all and only keep "en_US"? But I have no idea how such 
codes may look. Always like "xx_YY" or may that differ?
  - How about to show a flag/ensign instead of the code? Is there some stuff in 
KDE which may helpful?
  - Sure, then we have a less precise info there, but will someone use 
different variants of the same language at a time?
  
  Currently is a "special" dictionary not added to the menu. Mainly because it 
fit not into the "Preferred" section (and secondarily the needed member 
variable does right now not exist). To have more than one group need some 
effort to handle. To rename the group may a way, but how?

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, 
bruns, demsking, cullmann, sars, dhaumann

D18125: KateStatusBar: Add dictionary button

[loh tar]

loh.tar created this revision.
loh.tar added a reviewer: KTextEditor.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
loh.tar requested review of this revision.

REVISION SUMMARY
  ...to indicate the current dictionary and for the handy access to the
  spell check menu, which is enhanced to offer the user preferred
  dictionaries

TEST PLAN
  From shot to shot is only the cursor moved. The dictionaries was set 
previously.
  F6537575: pic.png 

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/katestatusbar.cpp
  src/view/katestatusbar.h

To: loh.tar, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, 
bruns, demsking, cullmann, sars, dhaumann