lanoxx / tilda

A Gtk based drop down terminal for Linux and Unix
GNU General Public License v2.0
1.27k stars 162 forks source link

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

Closed demlak closed 3 years ago

demlak commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

lanoxx commented 3 years ago

image

demlak commented 3 years ago

nice.. but at first i meant this:

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

:smiley:

lanoxx commented 3 years ago

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

lanoxx commented 3 years ago

@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 commented 3 years ago

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

lanoxx commented 3 years ago

Pushed with a few small modifications:

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