Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nicer "find in files" output, using pango markup #154

Closed
wants to merge 1 commit into from

Conversation

martinbonnin
Copy link

  • highlights grep matches and also groups results by files.
  • msgwin_msg_add() api is unchanged but text is converted to markup
    internally.
  • added msgwin_msg_add_markup() and msgwin_msg_add_markup_no_validate().
  • added utils_utf8_make_valid() and force valid utf8 in the GTK
    widgets, fixes some warnings.
  • do not process too many grep results at once to give a chance
    to the UI to handle some events sometimes.

before:
before

after:
after

* highlights grep matches and also groups results by files.
* msgwin_msg_add() api is unchanged but text is converted to markup
internally.
* added msgwin_msg_add_markup() and msgwin_msg_add_markup_no_validate().
* added utils_utf8_make_valid() and force valid utf8 in the GTK
widgets, fixes some warnings.
* do not process too many grep results at once to give a chance
to the UI to handle some events sometimes.
@elextr
Copy link
Member

elextr commented Aug 15, 2013

I don't think this is acceptable as it stands because:

  1. it only marks matches to simple search patterns, not regexes, but the regex matches are the ones that really need marking since the match text is not exactly the same as the search pattern

  2. it combines two things in one, these should be split

a. forcing encoding to UTF-8 for the text being put in the window, something which should be done anyway

b. highlighting the matches

Given that its quite a big patch and it only partly addresses the problem I don't think its worth the effort in its current form.

@b4n
Copy link
Member

b4n commented Aug 15, 2013

Agreed with @elextr. I like some of the ideas there, but some splitting and polishing are required before a merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants