search

mate-desktop / mate-terminal

The MATE Terminal Emulator
http://www.mate-desktop.org
GNU General Public License v3.0
133 stars 73 forks source link

fix url highlighting due to deprecation of vte_terminal_match_add_gregex #330

Closed norbusan closed 4 years ago

norbusan commented 4 years ago

Fixes Issue 329

sc0w commented 4 years ago

I see in configure.ac VTE_REQUIRED=0.48

I think you don't need the checks

norbusan commented 4 years ago

Oh, so we could drop support for the old way completely, that would simplify the patch.

Should I update it?

sc0w commented 4 years ago

yes, please

norbusan commented 4 years ago

Ok, I force pushed now with the checks removed and all switched to VteRegexp

sc0w commented 4 years ago

New warning detected in this PR:

terminal-screen.c: In function 'terminal_screen_class_init':
terminal-screen.c:596:15: warning: assignment to 'VteRegex **' {aka 'struct _VteRegex **'} from incompatible pointer type 'GRegex **' {aka 'struct _GRegex **'} [-Wincompatible-pointer-types]
  596 |  skey_regexes = g_new0 (GRegex*, n_skey_regexes);
      |               ^
norbusan commented 4 years ago

@sc0w Thanks, I missed that one during the simplification (VTE check removal), pushed a new commit that changes the type.

sc0w commented 4 years ago

now I noticed these warnings, but we have similar warnings in master

terminal-screen.c:186:2: warning: missing initializer for field 'flags' of 'TerminalRegexPattern' {aka 'const struct <anonymous>'} [-Wmissing-field-initializers]
  186 |  { "s/key [[:digit:]]* [-[:alnum:]]*",         FLAVOR_AS_IS },
      |  ^
terminal-screen.c:164:10: note: 'flags' declared here
  164 |  guint32 flags;
      |          ^~~~~

terminal-screen.c:187:2: warning: missing initializer for field 'flags' of 'TerminalRegexPattern' {aka 'const struct <anonymous>'} [-Wmissing-field-initializers]
  187 |  { "otp-[a-z0-9]* [[:digit:]]* [-[:alnum:]]*", FLAVOR_AS_IS },
      |  ^
terminal-screen.c:164:10: note: 'flags' declared here
  164 |  guint32 flags;
      |          ^~~~~
norbusan commented 4 years ago

@sc0w these warnings have been there since before, because the initializers for these structs always were missing the flag value. I guess putting a simple 0 at the end of all would silence the warnings.

sc0w commented 4 years ago

can you fix it here please?

norbusan commented 4 years ago

Ok, I'll add a commit fixing this. Please wait a bit.

norbusan commented 4 years ago

@sc0w I have pushed a trivial fix, but interestingly, I don't see the warning you pasted abvoe about missing initializers. Did you do something special when compiling?

Futhermore, there are more things to be fixed: The grexep stuff is still used in the search function, I will look into fixing the search, too.

norbusan commented 4 years ago

@sc0w I have now fixed also the search problem - at least for me searching works again. There is a strange warning message issued while searching:

(mate-terminal:54775): VTE-WARNING **: 07:49:15.285: (../src/vtegtk.cc:2477):void vte_terminal_search_set_regex(VteTerminal*, VteRegex*, guint32): runtime check failed: (regex == nullptr || _vte_regex_has_multiline_compile_flag(regex))

I checked the API, and passing NULL should be correct for clearing the regex, according the the vte api document. And I don't see any multline compile flag set.

Anyway, functionality-wise I can at least search with default settings, didn't try to do anything fancy, though.

sc0w commented 4 years ago

Did you do something special when compiling?

I just saw the travis logs (--enable-compile-warnings=maximum)

please, can you squash https://github.com/mate-desktop/mate-terminal/pull/330/commits/8e434e6fd564c212e33d256c1f5b174bef5483e8 https://github.com/mate-desktop/mate-terminal/pull/330/commits/90f80adc82ce9ba15d74d01948c4ace262296954 https://github.com/mate-desktop/mate-terminal/pull/330/commits/b171a434055957716420ece6caef9a9dcca15f8f into one unique commit?, and it will be ready to go.

I think https://github.com/mate-desktop/mate-terminal/pull/330/commits/3727f7c5467e4693dcc7640d7f2381a47bb48454 is unrelated and it can be handled in new PR, please open new PR with it.

norbusan commented 4 years ago

Hi @sc0w Done all that. Search related PR is here https://github.com/mate-desktop/mate-terminal/pull/332

norbusan commented 4 years ago

Thanks for merging! Now I am looking into all the other deprecated warnings, there are quite a lot of them ...

raveit65 commented 4 years ago

@norbusan Can this be used for distros with vte-0.58.3, eg. fedora 31? https://koji.fedoraproject.org/koji/buildinfo?buildID=1472915 My plan is to update fedora 31 with MATE 1.24 after fedora 32 is released in a few weeks.

norbusan commented 4 years ago

@raveit65 Yes, the functionality used is there since vte .48 afair, so if you have 0.58 then it should be no problem to use.