D7884: - Increase size of trailing mark

[Helio Castro]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:92ec63dd1b07: - Increase size of trailing mark (authored 
by helio).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7884?vs=20330&id=20903

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h
  src/view/kateviewinternal.cpp

To: helio, mwolff, kfunk, cullmann
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks, kevinapavew

D7884: - Increase size of trailing mark

[Christoph Cullmann]

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


  Then I would say => merge it.
  And thanks for the work!

REPOSITORY
  R39 KTextEditor

BRANCH
  big_trailing

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

To: helio, mwolff, kfunk, cullmann
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks, kevinapavew

D7884: - Increase size of trailing mark

[Helio Castro]

helio added a comment.


  In https://phabricator.kde.org/D7884#154232, @sars wrote:
  
  > I'm OK with an option for this. The only problem, with the option in the 
screen-shot, is that it could be interpreted as also effecting the tabulator 
mark.
  
  
  I did not changed the current behavior, if you keep in the same initial 
level, all stays same.

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks, kevinapavew

D7884: - Increase size of trailing mark

[Kåre Särs]

sars added a comment.


  I'm OK with an option for this. The only problem, with the option in the 
screen-shot, is that it could be interpreted as also effecting the tabulator 
mark.

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks

D7884: - Increase size of trailing mark

[Christoph Cullmann]

cullmann added a comment.


  Given it seems Atom even has the option to choose the sign for that, we can 
live with an option I assume (and the code for it is already written anyways).
  
  Others ok with that, too?

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks

D7884: - Increase size of trailing mark

[Helio Castro]

helio added a comment.


  Ping ?

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks

D7884: - Increase size of trailing mark

[Helio Castro]

helio updated this revision to Diff 20330.
helio added a comment.


  Rebase on current master

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7884?vs=19888&id=20330

BRANCH
  big_trailing

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h
  src/view/kateviewinternal.cpp

To: helio, mwolff, kfunk
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks

D7884: - Increase size of trailing mark

[Helio Castro]

helio added a comment.


  In https://phabricator.kde.org/D7884#148489, @cullmann wrote:
  
  > I would rather go with the "make the markers much larger but less contrast" 
solution than a setting for their size.
  >  Perhaps we really just need to look at how other editors do that.
  >  Eye-cancer like markers are no solution either, but I agree that if people 
turn them on, they should be able to spot them easily.
  
  
  And then will end up with the regular "this size is not good to me, can you 
make larger/smaller ?".
  Unfortunately, no size fits all, so i made the reasonable assumption to go 
from the original size to the near character size. 
  I honestly think that this will cover the 99% of users that will need this 
feature.
  
  This is how it appears on vim:
  F3943600: on_vim.png 
  
  We don't have the option on gedit.
  
  On Atom, we can't change color, we just can change the character of 
"Invisibles", which makes very bad at least on my screen:
  F3944421: atom.png 

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks

D7884: - Increase size of trailing mark

[Helio Castro]

helio added a comment.


  Fixed requests

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks

D7884: - Increase size of trailing mark

[Helio Castro]

helio updated this revision to Diff 19888.
helio marked 3 inline comments as done.
helio added a comment.


  - Moved calc logic to updateMarkerSize
  - Fix identation
  - Make value properly float
  - Update view when apply pressed

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7884?vs=19786&id=19888

BRANCH
  big_trailing

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h
  src/view/kateviewinternal.cpp

To: helio, mwolff, kfunk
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks

D7884: - Increase size of trailing mark

[Christoph Cullmann]

cullmann added a comment.


  I would rather go with the "make the markers much larger but less contrast" 
solution than a setting for their size.
  Perhaps we really just need to look at how other editors do that.
  Eye-cancer like markers are no solution either, but I agree that if people 
turn them on, they should be able to spot them easily.

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks

D7884: - Increase size of trailing mark

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> katerenderer.cpp:272
> +
> +pen.setWidthF(spaceWidth() / (3.5 - (m_markerSize * 0.5)));
>  pen.setCapStyle(Qt::RoundCap);

You can move width calculation in setmarkersize to safe unwanted calculations.

> katerenderer.h:406
>  bool m_showSpaces;
> + uint m_markerSize;
>  bool m_showNonPrintableSpaces;

Strange indent

> kateconfig.cpp:758
>  
> +void KateDocumentConfig::setMarkerSize(uint markerSize) {
> +if (m_markerSize == markerSize) {

function()
{
}

> kateconfig.h:242-243
>  
> + void setMarkerSize(uint markerSize);
> + uint markerSize() const;
> +

indent

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: anthonyfieroni, dhaumann, sars, #frameworks, cullmann

D7884: - Increase size of trailing mark

[Helio Castro]

helio added a comment.


  Is not that simple.
  Change only the color is fine, but then the effect is almost imperceptible,
  Right now from home i'm using a 25' 2560x1440 resolution with fonts in size 
10. Is a regular monitor, not the most expensive. 
  Even changing the color a single pixel is barely visible. And yes, the fonts 
are.
  If i go to HiDPI screen this becomes totally impossible to see ( unless you 
have eagle eyes ). This cases using a light background or white.
  If you are using darker backgrounds, like i use Breeze Dark, a pixel even on 
the most bright red color still mixed up. So, no good.
  
  To make my case mor appealing, all the other tools that we use, from simple 
vim to gerrit revision system shows a big mark, way more visible and this is 
essential to use to fix on the editor side or reject in gerrit side. So the 
only way that makes impossible to see the characters visible is kate. Assuming 
the current state is "good enought" of all cases was not a true statement

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: dhaumann, sars, #frameworks, cullmann

D7884: - Increase size of trailing mark

[Dominik Haumann]

dhaumann added a comment.


  Kate already has many options, so we try to be careful with adding even more 
options.
  
  What I would like to clarify first is the use case: If I understand 
correctly, the trailing space visualization is too decent for you. The decency 
is indeed intended, since the trailing spaces should not stand out more than 
the actual code itself.
  
  You are trying to fix this by increasing the visual appearance of the 
trailing spaces.
  
  Instead, would it also be an option for you to simply change the color of the 
trailing spaces? This can be configured in the Fonts & Colors config page.
  
  Would another solution be to add a high contrast color scheme?
  
  The point is: the colors are configurable because we can never fulfill 
everyone's needs. So simply charging the color to your own liking would be the 
preferred solution.

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: dhaumann, sars, #frameworks, cullmann

D7884: - Increase size of trailing mark

[Helio Castro]

helio added a comment.


  F3921383: configsize.png 

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: sars, #frameworks, cullmann, dhaumann

D7884: - Increase size of trailing mark

[Helio Castro]

helio updated this revision to Diff 19786.
helio added a comment.


  - Make size configurable keeping the original default

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7884?vs=19686&id=19786

BRANCH
  big_trailing

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h

To: helio, mwolff, kfunk
Cc: sars, #frameworks, cullmann, dhaumann

D7884: - Increase size of trailing mark

[Kevin Funk]

kfunk added a comment.


  In https://phabricator.kde.org/D7884#147087, @sars wrote:
  
  > I think "spaceWidth() / 2" goes a little bit too far.
  
  
  I agree. The 'before' screenshot looks better in my eyes. The trailing mark 
indicator is supposed to be a *subtle* hint, not something causing eye cancer 
to the casual reader :)
  
  > Shouldn't spaceWidth() fix the HiDPI problem?

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: sars, #frameworks, cullmann, dhaumann

D7884: - Increase size of trailing mark

[Kåre Särs]

sars added a comment.


  I think "spaceWidth() / 2" goes a little bit too far.
  
  Shouldn't spaceWidth() fix the HiDPI problem?

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: sars, #frameworks, cullmann, dhaumann

D7884: - Increase size of trailing mark

[Helio Castro]

helio added a comment.


  Result of change using kdevelop
  F3918646: screenshot.png 

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: #frameworks, cullmann, sars, dhaumann

D7884: - Increase size of trailing mark

[Helio Castro]

helio updated this revision to Diff 19686.
helio added a comment.


  - Using same code guideline as previous entry

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7884?vs=19685&id=19686

BRANCH
  big_trailing

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: helio, mwolff, kfunk
Cc: #frameworks, cullmann, sars, dhaumann

D7884: - Increase size of trailing mark

[Kevin Funk]

kfunk added a comment.


  Screenshots please, for changes like this.

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: #frameworks, cullmann, sars, dhaumann

D7884: - Increase size of trailing mark

[Helio Castro]

helio added reviewers: mwolff, kfunk.

REPOSITORY
  R39 KTextEditor

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

To: helio, mwolff, kfunk
Cc: #frameworks, cullmann, sars, dhaumann

D7884: - Increase size of trailing mark

[Helio Castro]

helio created this revision.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  On HiDPI screens and/or usage of small fonts current size of marker is
  so small making it almost invisible, even with a saturated color choice.
  This is specially visible on darker themes.

REPOSITORY
  R39 KTextEditor

BRANCH
  big_trailing

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: helio
Cc: #frameworks, cullmann, sars, dhaumann