google-code-export / notepad2-mod

Automatically exported from code.google.com/p/notepad2-mod
Other
2 stars 1 forks source link

Mark Occurrences #19

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hello,
do you plan to add the following two features:
1. XML/HTML tags match - the same as brace highlight/match but for XML tags. 
For example when the cursor is over a <tag> then highlight </tag>
2. Highlight all occurrences  - when a keyword or any text is selected 
highlight all occurrences in the document. This is similar to notepad++, VS, 
eclipse, etc.

Thanks

Original issue reported on code.google.com by al.le...@gmail.com on 12 Apr 2011 at 2:36

GoogleCodeExporter commented 9 years ago
From the project page:

"If you find any bugs or have any suggestions for the implemented lexers (and 
not only) feel free to provide patches."

So ask the developer of Notepad2.

Original comment by XhmikosR on 19 Apr 2011 at 10:48

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Thanks for the response. 

I contacted Florian few months ago, but unfortunately he could not commit 
when/if these features would be added to Notepad2.

Meanwhile, I downloaded you source code and implemented the "highlight all 
occurrences" feature myself :)
If you are interested in adding it to Notepad2-mod project I'll be happy to 
contribute my code.
How do I provide a patch?

Original comment by al.le...@gmail.com on 20 Apr 2011 at 1:52

GoogleCodeExporter commented 9 years ago
Use your svn client and choose to create a patch against the latest trunk.

Please make sure that there's no impact of your changes to the rest of the code 
and that everything works fine like before (I will test your patch too, but 
since you created, you should know better).

Original comment by XhmikosR on 20 Apr 2011 at 1:55

GoogleCodeExporter commented 9 years ago
Great, here is my patch attached.
Changes are:
- added new Mark Occurrences option in the View menu
- if enabled whenever a text/word is selected in the editor all occurrences are 
highlighted in the entire text
- option is saved in the ini file

Highlighting style is hardcoded - blueish with round corners :) This looks nice 
in the existing highlighting schemes, but could be made configurable later.

Original comment by al.le...@gmail.com on 20 Apr 2011 at 2:43

Attachments:

GoogleCodeExporter commented 9 years ago
Attached is your patch with one change: I only use spaces and never tabs for 
indentation. So use this patch for future changes.

Now, here are my thoughts:
1) I don't like the fact that if I double click again on the same keyword it's 
not unhighlighted. I have to double click on the whitespace for that to happen.
2)I'm not sure that this light blue color is good. I mean it might not be very 
distinctive in all cases but for now it should do the job.
3) If a keyword is highlighted and you try to select it, then it gets 
unhighlighted. If this is intended I'm pretty sure it's not good. If not maybe 
you should address this problem. For example, Notepad++ unhighlights the 
keyword when you click anywhere. My suggestion would be to unhighlight the 
keyword when you double click again on any highlighted instance only.

Let me know what you think.

Original comment by XhmikosR on 20 Apr 2011 at 4:09

Attachments:

GoogleCodeExporter commented 9 years ago
Oh, and maybe add a keyboard shortcut to toggle the option to Mark Occurrences 
(not sure if this should be renamed to something more descriptive).

Original comment by XhmikosR on 20 Apr 2011 at 4:12

GoogleCodeExporter commented 9 years ago
Here is the final update. 
Changes are:
- No more TABs in code
- Highlighting color is now configurable in the menu. Options are Red, Green, 
Blue, Off. Default (on fresh install) is Blue
- Highlighting now works exactly the same way as in notepad++. When you click 
anywhere all matches are un-highlighted.
- Performance optimization - only first 500 matches are highlighted. Who would 
need more for casual source code editing?!
- Performance optimization - search for matches only on edit event (the same as 
in brace match)

Well, that's it! The way it is now works perfectly for me, and I guess, others 
will find it usable as well.

Cheers!

Original comment by al.le...@gmail.com on 21 Apr 2011 at 11:05

Attachments:

GoogleCodeExporter commented 9 years ago
UPDATE: Sorry, I could not come up with a sensible shortcut combo for this. 
Besides, I'm not sure if this feature requires fast access - you just set it 
once and forget it :)

Original comment by al.le...@gmail.com on 21 Apr 2011 at 11:09

GoogleCodeExporter commented 9 years ago
Great work, thank you very much for your contribution!

Is there any chance that you reconsider the 500 matches limit and the way the 
words are unhighlighted? Unhighlight the word only if one double clicks again 
on any highlighted word.

Original comment by XhmikosR on 21 Apr 2011 at 11:20

GoogleCodeExporter commented 9 years ago
As for the keyboard shortcut, I agree with you now. The way it is implemented 
we could only add a keyboard shortcut for toggling the option later.
As for the 500 limit, I suggest that there is no limit. I mean, what's the 
performance penalty after all?

Thanks one more time for your contribution, I'll commit your patch when we 
figure out this couple of things.

Original comment by XhmikosR on 21 Apr 2011 at 11:25

GoogleCodeExporter commented 9 years ago
Here is the thing with the 500 limit.
If you open up a binary file (say Notepad2.exe itself) you'll see a lot of 
[null]s and other unreadable characters. There are thousands of them. If you 
select one of these [null]s you'll see why I introduced this limit. On my 4 
core CPU notepad2 hung for like 1 minute while searching for the thousands of 
matches to highlight. I understand that no one would intentionally do (or need) 
this but again it is very frustrating. This feature is useful when editing 
source code to quickly see where a variable is used throughout the code. I 
cannot think of a scenario where a variable would be used more than 500 times 
in a single file. I could safely double this limit, but I believe a limit MUST 
be set.

Now, as for the highlighting behavior. Let's say I change it as you suggest. 
Then I select a word (or double click on it which is actually the same as 
selecting it with the keyboard shift+arror keys) - all matches are highlighted. 
Now, in order to have new matches highlighted I have to first select again any 
highlighted occurrence and then select the new word I want to have matched. 
What I'm trying to say is that it is not about double-clicking at all - it is 
about text selection. The way it is now I could even select a part of a word 
and have this specific text highlighted (as a whole word) in the file.

What do you think?

Original comment by al.le...@gmail.com on 21 Apr 2011 at 12:12

GoogleCodeExporter commented 9 years ago
Regarding the limit, as you say, that would be a very stupid thing to do. But 
since it might happen then I guess a limit should exist. So we agree there. But 
500 occurrences seems not sufficient. 1000 should be enough.

Regarding the highlighting behavior. I think this way it's more "aggressive" 
and it can be quite annoying. That's why I said that I like better to toggle 
the highlighting only if one double clicks and selects a word. BTW, 
highlighting part of a word doesn't work for me so it must be a limitation in 
scintilla. Which makes my point more valid regarding the selection of a word. 
Double click a word-->The user explicitly expressed their will to highlight the 
word.

On another note, the green color should be different from the green color the 
bookmarks use otherwise it's not distinctive, see the attached screenshot.

Original comment by XhmikosR on 21 Apr 2011 at 12:34

Attachments:

GoogleCodeExporter commented 9 years ago
Ok, your suggestion to increase the limit is accepted - it is now 1000. Also, 
the green is now greener :)
See the attached screen to see what I mean about the partial selection match - 
"blah" which is not a whole word is selected and all "blah"s which are whole 
words are matched.
Maybe, what you suggest might be useful somehow, but I think it is a completely 
different mode for this feature. I will not implement it, at least not now. I 
believe, however, it should be made optional in the menu - "Sticky mode" or 
something.
Anyway, thanks for the constructive suggestions :)

Original comment by al.le...@gmail.com on 21 Apr 2011 at 1:37

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for your time you spent on this. I'll commit it as soon as possible. :)

I wonder in the following situation:

SaveSettings=1
SaveRecentFiles=0

Shouldn't the "Save" be highlighted if I choose to do so? Isn't this possible 
or it's a limitation?

Original comment by XhmikosR on 22 Apr 2011 at 12:04

GoogleCodeExporter commented 9 years ago
Committed in r479.

Original comment by XhmikosR on 27 Apr 2011 at 12:38

GoogleCodeExporter commented 9 years ago
Hello, I made few small changes:
- highlighting is only triggered when the selected text does not contain spaces 
or tabs or new lines etc. This should address the "glitch" reported in issue #22
- I moved the menu option just below the visual brace matching and highlight 
current line. I think it makes more sense there
- Red and blue colors are now slightly more intense. Since I started using it, 
I noticed that on some lcd screens they were not that "visible"

I am attaching two patches, because I am not sure what your update process it - 
one with differences to r476 and one compared to r486. 
Cheers!

Original comment by al.le...@gmail.com on 9 May 2011 at 9:46

Attachments:

GoogleCodeExporter commented 9 years ago
Hello. I only need the patch for the latest rev.

Now here is what I have noticed and I think it needs to be fixed. The words are 
not highlighted if the letter case isn't the same. For example: "For" and "for" 
should both be highlighted yet they aren't.

Attached is your r486 patch, with the only change the spaces in the rc file, so 
please use that as a base for your future patches.

PS. If you feel like working on more things please let me know so that I can 
grant you svn access.

Thanks for your contribution.

Original comment by XhmikosR on 9 May 2011 at 12:41

Attachments:

GoogleCodeExporter commented 9 years ago
Hey, I had some time today to work on this. I think it's getting more advanced 
than I initially thought :)
Here is the change log:
- added Match case option (this controls whether to mark "For" if "for" is 
selected)
- added Match only whole words option (this controls whether to mark Save in 
SaveSettings when Save is selected)
These are both checked by default.

As for the SVN access, I'm not sure if I'll have much time to implement any 
bigger enchantments. Of course, if you don't want to merge my patches now and 
then, I could commit them directly myself. 
Anyway, if I come up with other interesting features will let you know for sure.

Original comment by al.le...@gmail.com on 10 May 2011 at 1:12

Attachments:

GoogleCodeExporter commented 9 years ago
Hmm, I'm not sure about the last patch. There shouldn't be an option for this. 
We don't need to overcomplicate things. Just mark the words regardless of the 
case. If there is demand you can add it later but I'd like things simple.

I'll send you an email about the svn access.

Original comment by XhmikosR on 10 May 2011 at 1:24

GoogleCodeExporter commented 9 years ago
Hi,
first at all I want to say thanks for this patch. It was almost the last thing 
I missed in Notepad2.

Maybe nobody cares, but I wanted to say that I like the last patch. I think 
these to options are very helpful and do not make the feature complicated. I 
would appreciate if it finds it way into Notepad2.

Anyways, thank you both for the good work!

Best regards

Original comment by Volk...@gmail.com on 10 May 2011 at 2:33

GoogleCodeExporter commented 9 years ago
I totally disagree. They complicate the feature with no reason. I haven't found 
any other text editor which does this which proves my point.
Like I said, I suggest that you drop the different settings so that it goes in 
and if there's enough demand you can commit the various settings change.

Original comment by XhmikosR on 10 May 2011 at 2:47

GoogleCodeExporter commented 9 years ago
Let's do it this way - these settings could be removed from the menu, but left 
configurable in the .ini file for some super user to play with. 
What do you think?

Original comment by al.le...@gmail.com on 10 May 2011 at 5:32

GoogleCodeExporter commented 9 years ago
Let's just make the "Match Case" option disabled by default.

I also find the green color to be more distinctive than the blue one so I made 
it the default.

Attached is the modified patch for r492 with the above changes.

Let me know what you think.

PS. What do you think about changing the order in this switch statement to be 
like the Menu options are in r492:

  switch (iMarkOccurrences)
  {
    case 0: i = IDM_VIEW_MARKOCCURRENCES_OFF;break;
    case 1: i = IDM_VIEW_MARKOCCURRENCES_BLUE;break;
    case 2: i = IDM_VIEW_MARKOCCURRENCES_GREEN;break;
    case 3: i = IDM_VIEW_MARKOCCURRENCES_RED;break;
  }

I suppose this would mean that the setting will be reset for people who use the 
same ini file, i.e. upgrade from an older version.

Original comment by XhmikosR on 10 May 2011 at 6:39

Attachments:

GoogleCodeExporter commented 9 years ago
Updated patch is attached. I changed the accelerator keys for the new menu 
items along with the case so that it matches the rest of the menus.

BTW, maybe the red should be a little more red like you did with yellow?

Also, is the situation in the attached image normal? Match case is disabled, 
whole words enabled.

Original comment by XhmikosR on 10 May 2011 at 10:01

Attachments:

GoogleCodeExporter commented 9 years ago
Hi, I'm ok with changing the order. My logic was to have it red, green, blue as 
in RGB.
Now for the screenshot, it is normal. In this case a whole word is matched - 
"update" :) In this case if you select the matched "update" then 
UpdateAtStartup won't be selected because it isn't a whole word itself.
I was thinking the same about the colors. How about making them all as intense 
as the green is?

Original comment by al.le...@gmail.com on 11 May 2011 at 8:04

GoogleCodeExporter commented 9 years ago
Hi. Yes, I agree with the colors intensity.

Original comment by XhmikosR on 11 May 2011 at 12:51

GoogleCodeExporter commented 9 years ago
Here's the patch with all colors boosted up and changes to make the order you 
suggested work. If someone has saved settings with the old order it will be 
preserved.

Original comment by al.le...@gmail.com on 12 May 2011 at 12:28

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Thanks for your contribution. Everything seems to work like it should.

Unfortunately I noticed the problem shown in the attached image. It happens 
with r493 too, so it's not something introduced with this patch. Do you have 
any idea what might be causing it?

PS. I have sent you an email regarding svn access, not sure if you got it.

Original comment by XhmikosR on 12 May 2011 at 12:44

Attachments:

GoogleCodeExporter commented 9 years ago
BTW, I start to like the new blue color more... Maybe we should use that as the 
default one :P

Original comment by XhmikosR on 12 May 2011 at 12:49

GoogleCodeExporter commented 9 years ago
He he, yep, I like the blue one better, too! 
You could put it default in notepad2.c iMarkOccurrences = 
IniSectionGetInt(pIniSection,L"MarkOccurrences",3);

The issue in the screenshot is really bad! However, it has nothing to do with 
this feature. Try turning it off and you'll see that it is still reproducible. 
I even disabled mark occurrences at code level the it behaves exactly the same. 
It's definitely an issue with the bookmarks code. I could take a look at it 
when I find some time.
By the way, I got your email but have not tried to commit anything yet, thanks.

Original comment by al.le...@gmail.com on 12 May 2011 at 1:13

GoogleCodeExporter commented 9 years ago
All right, patch committed with blue being still the default. Many thanks for 
all of your work!

Now about the bookmarks, you are right. It happens in the original Bookmark 
Edition build too. The problem is if the last word in a sentence is 
selected/highlighted etc.
I'd really appreciate it if you could have a look. All of the bookmark edition 
code is in #ifdefs BOOKMARK_EDITION. The changes made to that code are minimal, 
like compiler warnings fix and a couple of other things in r274 and r279.

PS. Now you have svn access (I forgot to actually add you :P)

Original comment by XhmikosR on 12 May 2011 at 3:34

GoogleCodeExporter commented 9 years ago
Can you create a new issue for the XML tags request so that we can close this 
one?
I'll create a new one for the bookmarks highlight problem since it doesn't 
belong here.

Original comment by XhmikosR on 12 May 2011 at 9:39

GoogleCodeExporter commented 9 years ago
Renaming and closing this issue now that Mark Occurrences are in the SVN.
New Issue #27 is for the XML/HTML tags match request.

Original comment by XhmikosR on 16 May 2011 at 3:23