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

Url-parsing includes closing squarebrackets, also when the openen one is before #405

Closed
demlak opened this issue Feb 9, 2020 · 13 comments
Closed

Comments

@demlak
Copy link

demlak commented Feb 9, 2020

Hi,
i'm using tilda 1.5.0 and when there are urls detected, the detection does not reflect beginning of square brackets before URL.

for example here.. the link! [https://fdsgdf.com/gfhsjfg/fdh/hfjkdg] take a look!
is detected as https://fdsgdf.com/gfhsjfg/fdh/hfjkdg]

i'm not sure if this is a real bug.. or just not realy good to handle

lanoxx added a commit that referenced this issue Nov 1, 2020
This patch implements a match registry that can provide a set of
different regular expressions to be registered in the terminal.

This not only allows us to perform correct matching of more complex URLs
but also file links, email addresses, VoIP links, man pages and numbers.

LICENSE NOTES

This includes code taken from gnome-terminal, which is licensed under
GPLv3. According to the [compatibility matrix][1] of the GNU GPL license
FAQ this means that existing code in tilda can remain "GPLv2 or later"
licensed, but the combined code is now licensed under "GPLv3 or later".

Fixes: #405

[1]: https://www.gnu.org/licenses/gpl-faq.html#AllCompatibility
lanoxx added a commit that referenced this issue Nov 8, 2020
This patch implements a match registry that can provide a set of
different regular expressions to be registered in the terminal.

This not only allows us to perform correct matching of more complex URLs
but also file links, email addresses, VoIP links, man pages and numbers.

Fixes: #405
@lanoxx
Copy link
Owner

lanoxx commented Nov 22, 2020

I have been working an update of the matching code which you can find on the wip-pcre branch. The new code is now able to match also Numbers, Email addresses, and file URLs, so I had to change the functionality of the context menu a little and I would like to receive some feedback on the UI before I include this into the master branch.

The matching code now detects the typo of the matched token and updates the context menu according to the match type. Depending on the type it is possible to copy the matched token or also open it.

Email matched:
tilda-context-menu-email

File URI matched:
tilda-context-menu-file

Number matched (no open link):
tilda-context-menu-number

URL matched:
tilda-context-menu-url

@demlak
Copy link
Author

demlak commented Nov 22, 2020

tried to compile..

# ../autogen.sh --prefix=/usr
This script will generate the initial build system files to compile
tilda successfully. When done it will call the ./configure script
and pass on any options which you passed to this script
See ./configure --help to know which options are available

When it is finished, take the usual steps to install:
make
make install

Generating build system configuration for tilda, please wait...

autoreconf: Entering directory `.'
autoreconf: running: autopoint --force
autoreconf: running: aclocal --force -I m4 ${ACLOCAL_FLAGS}
autoreconf: configure.ac: tracing
autoreconf: configure.ac: not using Libtool
autoreconf: running: /usr/bin/autoconf --force
autoreconf: running: /usr/bin/autoheader --force
autoreconf: running: automake --add-missing --force-missing
configure.ac:149: warning: The 'AM_PROG_MKDIR_P' macro is deprecated, and its use is discouraged.
configure.ac:149: You should use the Autoconf-provided 'AC_PROG_MKDIR_P' macro instead,
configure.ac:149: and use '$(MKDIR_P)' instead of '$(mkdir_p)'in your Makefile.am files.
autoreconf: Leaving directory `.'
Running configure...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /usr/bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking whether to enable maintainer-specific portions of Makefiles... yes
checking for glib-mkenums... /usr/bin/glib-mkenums
checking for glib-compile-resources... /usr/bin/glib-compile-resources
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking whether make supports the include directive... yes (GNU style)
checking dependency style of gcc... gcc3
checking whether NLS is requested... yes
checking for msgfmt... /usr/bin/msgfmt
checking for gmsgfmt... /usr/bin/msgfmt
checking for xgettext... /usr/bin/xgettext
checking for msgmerge... /usr/bin/msgmerge
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking for ld used by GCC... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for shared library run path origin... done
checking how to run the C preprocessor... gcc -E
checking for grep that handles long lines and -e... /usr/bin/grep
checking for egrep... /usr/bin/grep -E
checking for CFPreferencesCopyAppValue... no
checking for CFLocaleCopyCurrent... no
checking for GNU gettext in libc... yes
checking whether to use NLS... yes
checking where the gettext function comes from... libc
checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for GTK... yes
checking for VTE... yes
checking for LIBCONFUSE... yes
checking for X11... yes
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking fcntl.h usability... yes
checking fcntl.h presence... yes
checking for fcntl.h... yes
checking malloc.h usability... yes
checking malloc.h presence... yes
checking for malloc.h... yes
checking for stdlib.h... (cached) yes
checking for string.h... (cached) yes
checking for strings.h... (cached) yes
checking sys/ioctl.h usability... yes
checking sys/ioctl.h presence... yes
checking for sys/ioctl.h... yes
checking for unistd.h... (cached) yes
checking for an ANSI C-conforming const... yes
checking for pid_t... yes
checking for size_t... yes
checking for stdlib.h... (cached) yes
checking for GNU libc compatible malloc... yes
checking for working strtod... yes
checking for mkdir... yes
checking for strcasecmp... yes
checking for strchr... yes
checking for strncasecmp... yes
checking for strstr... yes
checking for strtol... yes
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating po/Makefile.in
config.status: creating config.h
config.status: executing depfiles commands
config.status: executing po-directories commands
config.status: creating po/POTFILES
config.status: creating po/Makefile

              tilda 1.6-alpha
              ===============

        prefix:                        /usr
        datarootdir:                   ${prefix}/share
        datadir:                       ${datarootdir}
        pkgdatadir:                    ${datarootdir}/tilda
        source code location:          ..
        compiler:                      gcc
        cflags:                        -std=c99 
        Maintainer mode:               yes
        VTE:                           
        Use *_DISABLE_DEPRECATED:      

# LANG=en_US.UTF-8 make
  GEN      src/glade-resources.h
  GEN      src/glade-resources.c
  GEN      src/tilda-enum-types.c
  GEN      src/tilda-enum-types.h
make  all-recursive
make[1]: Entering directory '/home/demlak/Downloads/tilda-wip-pcre/build'
Making all in po
make[2]: Entering directory '/home/demlak/Downloads/tilda-wip-pcre/build/po'
make tilda.pot-update
make[3]: Entering directory '/home/demlak/Downloads/tilda-wip-pcre/build/po'
make[3]: *** No rule to make target '../../src/menu.ui', needed by 'tilda.pot-update'.  Stop.
make[3]: Leaving directory '/home/demlak/Downloads/tilda-wip-pcre/build/po'
make[2]: *** [Makefile:237: ../../po/tilda.pot] Error 2
make[2]: Leaving directory '/home/demlak/Downloads/tilda-wip-pcre/build/po'
make[1]: *** [Makefile:1130: all-recursive] Error 1
make[1]: Leaving directory '/home/demlak/Downloads/tilda-wip-pcre/build'
make: *** [Makefile:577: all] Error 2

@lanoxx
Copy link
Owner

lanoxx commented Nov 22, 2020

I removed the menu.ui files from the sources since the menu is now created dynamically from the code. It seems I forgot to remove it from the translation scripts. I am not quit sure why that did not cause an error on my system. I pushed an update to the branch, please try again.

@demlak
Copy link
Author

demlak commented Nov 22, 2020

compiling worked..
matching seems also to work.. nice!

eventually link-klicking is broken?
usualy i just set "firefox" as webbrowser option and it worked on other tilda versions.
Here i have to right click -> open link
Direct clicking link, does not open link

@lanoxx
Copy link
Owner

lanoxx commented Nov 23, 2020

I added a change which requires that the control key is pressed when a link is clicked. That has been requested in another bug for some time. This change should help to avoid accidentally clicking on the link when trying to copy some text, but it also makes it a bit less discoverable.

For the record, here is a snipped I used to test the matching functionality, that contains examples for Number, URL without schema, URL with Schema, email without prefix, email with mailto prefix and a file URI:

echo 12345;                       # Copy Number
echo www.google.com;              # Copy / Open Link
echo https://google.com;          # Copy / Open Link
echo email@example.com;           # Copy / Send Email
echo mailto:example@example.com;  # Copy / Send Email
echo file:///usr/                 # Copy / Open File

You probably already noticed that the labels adjust and change between Open ... and Send .... Also the Send option is omitted for the number match, which means the context menu is one entry smaller for number matches than for other matches and two entries smaller if no match is detected. I am still considering to always show all entries but make them deactivated to ensure the context menu always has the same size. Let me know if you have any preferences.

@demlak
Copy link
Author

demlak commented Nov 23, 2020

i think, users are very diverse. Maybe it's a good choice to not declare for all, but implement option to modify? this would reflect different workflows of people.

@lanoxx
Copy link
Owner

lanoxx commented Nov 28, 2020

Added configurable options for the context menu in commit 37aacb1.

@lanoxx
Copy link
Owner

lanoxx commented Nov 28, 2020

image

@demlak
Copy link
Author

demlak commented Nov 28, 2020

nice.. but at first i meant this:

I added a change which requires that the control key is pressed when a link is clicked.

😃

@lanoxx
Copy link
Owner

lanoxx commented Dec 13, 2020

Issue #368 tracks match activation handling, I have pushed a commit to master to fix that issue.

lanoxx added a commit that referenced this issue Dec 13, 2020
This patch implements a match registry that can provide a set of
different regular expressions to be registered in the terminal.

This not only allows us to perform correct matching of more complex URLs
but also file links, email addresses, and numbers.

Fixes: #405
@lanoxx
Copy link
Owner

lanoxx commented Dec 13, 2020

@demlak I have rebased this branch on master and cleaned up the code a little. It now also properly handles OSC 8 hyperlinks (i.e. printf '\e]8;;http://example.com\e\\This is a link\e]8;;\e\\\n'). I think the code is ready to be merged, if you could give it a final test and let me know if you notice any issues that would be great. If you can confirm that it works fine, then I will merge it into master.

@demlak
Copy link
Author

demlak commented Dec 13, 2020

Awesome!
i just took a short view and did not see any issues

@lanoxx lanoxx closed this as completed in 2bd442f Dec 14, 2020
@lanoxx
Copy link
Owner

lanoxx commented Dec 14, 2020

Pushed with a few small modifications:

  • Added license clarifications and headers, since the new tilda-regex.h header is GPLv3 (or later).
  • Fixed a match opening issue for links missing the http:// schema prefix (i.e. www.example.com).

This is now in master. Thanks for helping to test this.

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

No branches or pull requests

2 participants