Code::Blocks Forums

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: danselmi on July 29, 2013, 11:22:39 pm

Title: Move Occurrence highlighting from core into a plugin
Post by: danselmi on July 29, 2013, 11:22:39 pm
Hi

As mentioned here:
http://forums.codeblocks.org/index.php/topic,18070.msg123704.html#msg123704 (http://forums.codeblocks.org/index.php/topic,18070.msg123704.html#msg123704)
I moved the code of the occurrence highlighting from sdk and the mentioned plugin into a core-plugin. Patch is attached.

Does anybody have some time to

Regards,


daniel




[attachment deleted by admin]
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: Jenna on July 30, 2013, 01:07:13 am
I can do the automake-stuff (in fact I just started doing it).
I will also have a quick look into the sources/headers.
I will see if I can make iamges also. Most likely made of a screenshot, similar to IncSearch.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: Jenna on July 30, 2013, 01:53:54 am
Here comes a first addition to your patch.
It has to be applied after yours.

I can put it in one patch, but it will be more difficult to see the changes I made.

I did not (yet) update the wx2.9 project-files.

Hopefully tomorrow.
Now I go to bed.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: ollydbg on July 30, 2013, 04:01:14 am
Good work, I did not test the patch, just quick read the patch file:

Code
Index: src/plugins/occurrenceshighlighting/highlighter.cpp
===================================================================
--- src/plugins/occurrenceshighlighting/highlighter.cpp (revision 0)
+++ src/plugins/occurrenceshighlighting/highlighter.cpp (working copy)
@@ -0,0 +1,356 @@
+#include "Highlighter.h"
+
+#include <sdk.h> // Code::Blocks SDK
+//#include <editorcolourset.h>
+#include <cbstyledtextctrl.h>
+#include <cbcolourmanager.h>
Is it better to put sdk.h as the first include files?

Code
+#ifndef Highlighter_h
+#define Highlighter_h
and
Code
+#ifndef OCCURRENCESHIGHLIGHTING_H_INCLUDED
+#define OCCURRENCESHIGHLIGHTING_H_INCLUDED
I would suggest to use a uniform string. like XXXXX_H
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: MortenMacFly on July 30, 2013, 03:39:01 pm
Well I cannot apply Jens' patch (I assume its GIT based again), but the part of Daniel works fine for me so far. Nice work!
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: oBFusCATed on July 30, 2013, 03:40:24 pm
...(I assume its GIT based again)...
It is not in git format, looks like a valid svn patch.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: ollydbg on July 30, 2013, 03:53:53 pm
...(I assume its GIT based again)...
It is not in git format, looks like a valid svn patch.
Code
commit 76a459e2d6c0f8d420ac4bf20807abb73e02052e
Author: Jens Lody <jens@codeblocks.org>
Date:   Tue Jul 30 01:50:04 2013 +0200

    * additional patch for move of occurrences highlighting

Index: acinclude.m4
===================================================================
--- acinclude.m4
+++ acinclude.m4
@@ -170,6 +170,17 @@
  AC_MSG_RESULT(no)
Look at the first line, it should be a git patch, Jens may use some utility tool to translate this patch a little to make it svn compatible.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: oBFusCATed on July 30, 2013, 04:07:09 pm
Look at the first line, it should be a git patch, Jens may use some utility tool to translate this patch a little to make it svn compatible.
You can have whatever you want before the Index lines. The patch is in a format that is almost like a patch produced by svn.
If you remove the top lines you won't be able to guess that it has been produced with git.

The thing is that Morten uses some broken patch utility, so it is not a problem that the patch is broken, but the tool Morten is using is broken.

From what I remember it is pretty hard to find working patch utility for windows.  :'(
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: ollydbg on July 30, 2013, 04:18:16 pm
The thing is that Morten uses some broken patch utility, so it is not a problem that the patch is broken, but the tool Morten is using is broken.

From what I remember it is pretty hard to find working patch utility for windows.  :'(
I do not have any problem about the patch utility.
For the record, the most recent patch utility is in MSYS2 package
Quote
New MSYS2 snapshots:

32-bit:    x32-msys2-alpha-20130728.tar.xz  http://sourceforge.net/projects/msys2/files/Alpha-versions/32-bit/x32-msys2-alpha-20130728.tar.xz/download

64-bit:    x64-msys2-alpha-20130728.tar.xz  http://sourceforge.net/projects/msys2/files/Alpha-versions/64-bit/x64-msys2-alpha-20130728.tar.xz/download
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: MortenMacFly on July 30, 2013, 04:27:29 pm
The thing is that Morten uses some broken patch utility, so it is not a problem that the patch is broken, but the tool Morten is using is broken.
Nope - it works 100% if you use SVN diff. I'd say its just purely SVN oriented.

From what I remember it is pretty hard to find working patch utility for windows.  :'(
However, thats true as well. Thing is I don't want to use a console utility if I have a full-blown GUI-based IDE. Thats basically also the reason why I prefer Windows over Linux.

If I were a console guy and there were a nice utility I wouldn't mind at all and take the time to even compile patch myself. The point is: Its actually so easy to create as working SVN patch. So why do work-arounds?

I think we had this discussion now over and over again. So please, whenever I post "I cannot apply this" just take it as a hint that besides me probably other user cannot try what you've posted and you limit the testers. If thats fine with you, everything is alright!
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: oBFusCATed on July 30, 2013, 04:44:21 pm
However, thats true as well. Thing is I don't want to use a console utility if I have a full-blown GUI-based IDE. Thats basically also the reason why I prefer Windows over Linux.
Then report the problems to your IDE providers, as far as I know it is even a paid one.

The point is: Its actually so easy to create as working SVN patch. So why do work-arounds?
Because svn doesn't have local commits, yet! When they add local commits/branches to svn you'll see why we prefer to use git. :)
And it is not that easy if you don't have svn checkout. :)  But at least my script works fine, I don't know what Jens' is using though.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: MortenMacFly on July 30, 2013, 04:56:50 pm
Then report the problems to your IDE providers, as far as I know it is even a paid one.
I won't anymore for three reasons:
1.) Its only C::B where these issues occur and this is spare-time, so its not a show-stopper for me at all
2.) They usually want to have a small test-case and thats hard to do as I first need to find out what actually fails and I don't have time for it.
3.) The software was just sold to a larger company and this affects support which has basically stalled. Previously it was a flexible 2-3 employees (German) company, now its a large 100+ (US) company.

So in fact once my license has run out I will look for alternatives, if they are affordable.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: oBFusCATed on July 30, 2013, 05:06:17 pm
Fair enough. I suppose you can try one of the command line patch utilities in the mean time, so at least you have a working alternative for the case where your tool fails. :)
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: Jenna on July 30, 2013, 09:12:01 pm
I upload a new patch, that fixes the Makefile.am of the highlighte, it replaces the older version of my additional patch.
By the way: both of my patches apply fine with TortoisSVN.

If they don't apply with "Smart"SVN, I do not really care, sorry.
There is no way for me to fix or even test it.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: Jenna on July 31, 2013, 03:04:01 am
Next updated patch.
Should cover all project-files and update-scripts now.
Name changed back from CamelCase to all lower letteres, as we have it for all core-plugins, therefore also changed the case for the zip-file.

Windows files are not (yet) tested.
Linux files should build correctly now.


There is one issue with permanently highlighting.

The highlighting will occurr (most of the times) first after clicking into the file (if it is already opened and not the active one).
It's the same when permanently highlighting is turned off for a word: in actual file highlighting is removed immediately, in open files it is removed after clicking into the file at any place.

I can look into it tomorrow, if you want.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: Jenna on July 31, 2013, 10:56:32 am
There is one issue with permanently highlighting.

The highlighting will occurr (most of the times) first after clicking into the file (if it is already opened and not the active one).
It's the same when permanently highlighting is turned off for a word: in actual file highlighting is removed immediately, in open files it is removed after clicking into the file at any place.

I can look into it tomorrow, if you want.

This patch fixes it for me on linux.
I did not (yet) test the whole stuff on windows, so it might be a (wx)scintilla on linux/gtk issue only.
Code
Index: src/plugins/occurrenceshighlighting/highlighter.cpp
===================================================================
--- src/plugins/occurrenceshighlighting/highlighter.cpp
+++ src/plugins/occurrenceshighlighting/highlighter.cpp
@@ -33,7 +33,8 @@
     if ( Manager::Get()->GetEditorManager()->GetActiveEditor() != ctrl  ) return;
 
     // check the event type if it is an update event
-    if ( event.GetEventType() == wxEVT_SCI_UPDATEUI )
+    if ( event.GetEventType() == wxEVT_SCI_UPDATEUI ||
+         event.GetEventType() == wxEVT_SCI_PAINTED)
     {
         HighlightOccurrencesOfSelection(ctrl);
         OnEditorUpdateUI(ctrl);
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: MortenMacFly on August 01, 2013, 09:04:46 am
Next updated patch.
This one applies safely for me. I believe it covers 2+3?
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: Jenna on August 01, 2013, 10:00:13 am
Next updated patch.
This one applies safely for me. I believe it covers 2+3?
Yes, it does.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: Jenna on August 06, 2013, 10:31:56 pm
Any comments, issues or whatever ?

I personally think it should be committed to trunk, if everything works as expected.
Title: Re: Move Occurrence highlighting from core into a plugin
Post by: MortenMacFly on August 07, 2013, 07:26:08 am
Any comments, issues or whatever ?
Go ahead - works fine for me since day 1 posted here.