D25328: Always rehiglhight() after definition was changed

[Méven Car]

meven abandoned this revision.
meven added a comment.


  Fixed in thumnail creator in D25891 

REPOSITORY
  R216 Syntax Highlighting

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

To: meven, kossebau, cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars

D25328: Always rehiglhight() after definition was changed

[Méven Car]

meven added a comment.


  In D25328#564202 , @meven wrote:
  
  > In D25328#563637 , @cullmann 
wrote:
  >
  > > Hmm, the use case is that it re-highlights if you set an invalid 
definition?
  >
  >
  > Here it rehighlight if the definition has changed `definition() != def`.
  >  But since the current definition `m_definition` has initially the value 
`Definition()`, it is the same as the invalid default definition returned by 
`Repository::definitionForFileName` (set in `RepositoryPrivate::load` line 161) 
and does not call `rehighlight`.
  >
  > > Perhaps one should just trigger rehighlight here just for that case, too.
  >
  > So the fix does not really depend on the definition being valid or invalid.
  >
  > Perhaps there is a better way to fix this.
  
  
  Any thought from a SyntaxHighlighter maintainer ?
  I hope I have made the issue clear enough.

REPOSITORY
  R216 Syntax Highlighting

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

To: meven, kossebau, cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars

D25328: Always rehiglhight() after definition was changed

[Méven Car]

meven added a comment.


  In D25328#563637 , @cullmann wrote:
  
  > Hmm, the use case is that it re-highlights if you set an invalid definition?
  
  
  Here it rehighlight if the definition has changed `definition() != def`.
  But since the current definition `m_definition` has initially the value 
`Definition()`, it is the same as the invalid default definition returned by 
`Repository::definitionForFileName` (set in `RepositoryPrivate::load` line 161) 
and does not call `rehighlight`.
  
  > Perhaps one should just trigger rehighlight here just for that case, too.
  
  So the fix does not really depend on the definition being valid or invalid.
  
  Perhaps there is a better way to fix this.

REPOSITORY
  R216 Syntax Highlighting

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

To: meven, kossebau, cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars

D25328: Always rehiglhight() after definition was changed

[Christoph Cullmann]

cullmann added a comment.


  Hmm, the use case is that it re-highlights if you set an invalid definition?
  Perhaps one should just trigger rehighlight here just for that case, too.

REPOSITORY
  R216 Syntax Highlighting

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

To: meven, kossebau, cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars

D25328: Always rehiglhight() after definition was changed

[Méven Car]

meven added a comment.


  In D25328#563406 , @dhaumann wrote:
  
  > Can't you call `rehighlight()` yourself after calling `setDefinition()`?
  >
  > Btw, tanks for looking into this!
  
  
  I have done so in D25323 .
  But I meant to open this one as well to figure out which way to go, user side 
fix or lib fix ?
  I would favor the lib fix as it seems it might save some users the trouble of 
adding a workaround because no definition is found just to get highlight like 
D25323  does.
  Kate has such a workaround as well in KateSyntaxHighlight I believe.

REPOSITORY
  R216 Syntax Highlighting

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

To: meven, kossebau, cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars

D25328: Always rehiglhight() after definition was changed

[Dominik Haumann]

dhaumann added a comment.


  Can't you call `rehighlight()` yourself after calling `setDefinition()`?
  
  Btw, tanks for looking into this!

REPOSITORY
  R216 Syntax Highlighting

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

To: meven, kossebau, cullmann, vkrause
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars

D25328: Always rehiglhight() after definition was changed

[Christoph Cullmann]

cullmann added a comment.


  Hmm, I am not sure this doesn't regress cases that rely on setDefinition 
being "cheap" for the nop case.
  Is there no other way to trigger re-highlight for the wanted case?

REPOSITORY
  R216 Syntax Highlighting

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

To: meven, kossebau, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25328: Always rehiglhight() after definition was changed

[Méven Car]

meven edited the summary of this revision.
meven edited the test plan for this revision.

REPOSITORY
  R216 Syntax Highlighting

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

To: meven, kossebau, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25328: Always rehiglhight() after definition was changed

[Méven Car]

meven created this revision.
meven added reviewers: kossebau, cullmann, vkrause.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
meven requested review of this revision.

REVISION SUMMARY
  When setting definition, Rehighlight text even if the definition has not 
changed.
  Useful when the dummy default definition is set, so that highlight is run 
even in this case.

TEST PLAN
  ctest
  In dolphin, text files content is readable in thumbnails.
  
  BUG: 409380
  FIXED-IN: 5.65

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

AFFECTED FILES
  src/lib/syntaxhighlighter.cpp

To: meven, kossebau, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann