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

Enable Tab completion with favourites #89

Closed
wants to merge 10 commits into from
Closed

Enable Tab completion with favourites #89

wants to merge 10 commits into from

Conversation

msdemlei
Copy link

The main point in this pull request is to enable tab completion of items
in the list of favourites.

To do this, in commits f7c193c,
6f09b31, and
9473e2b I made the existing code for
handling the history generic enough so it can be used for the
favourites, too.

In commit 5da430d, the favourites list
is actually managed in memory; this results in a change in the favourite
file's on-disk format. There's code to heuristically decide which
format a concrete favourites list has, but I give you it's not perfect.

Then, in commit 785ae79 the in-memory
favourites are used to complete URIs.

Now, I like to match any part of a URI when completing. Hence, in
commit 537c2d3 I add a config option
complete_uri_anywhere defaulting to false (current xombrero behaviour).
If you set it to 1, URI expansion will happen based on matches anywhere
in the URIs.

Drawbacks/remaining problems:

load_pagelist_from_disk breaks in non-English locales, as has been the
case for loading history before, as strptime is locale-dependent. I'm
happy to clean this up at this point, but as (essentially) no new
breakage is introduced, I'd say it's an independent problem.

It would be good to be able to use the completion model from complete.h
to complete URLs in cmd_getlist rather than manually going through the
two pagelists (frankly, the fact that completion_add_uri didn't have an
effect on completing "open" was really confusing to me). Alas, I've not
found an API to get to the actual char_s from a GtkTreeView, so after
completion we'd have to free all the gchar_, which seemed an unnecessary
code complication.

Since I'm a professing git idiot, the pull request contains some commits
that shouldn't have gone in: bba1935
and 3342d90, plus the horrible mess
I've made today. Before I actually try the magic to fix the history or
create a synthetic branch, could you see if you can just cherry-pick the
actually useful commits?

@marcopeereboom
Copy link
Contributor

I think I like the gist of this but I would like to split this out and fix the remaining issues. If you are fighting git a lot I am ok with diffs in email too.

@msdemlei
Copy link
Author

msdemlei commented Apr 9, 2015

Hm -- so, the two issues I'm mentioning above are:

(1) the locale-dependent strptime -- that I'd gladly fix (though I
make no promises it's going to be next weekend). In case anyone is
aware of some function that parses C-locale ctimes regardless of
locale (webkit must have something like this, as ctimes are in HTTP
headers), I'll gladly take any advice.

(2) The trouble that the gtk completion model and the "command line"
completion use different mechanisms (which is true for current
xombrero, too) is, I think, not easy to fix due to the mismatch
between the GtkTreeView API and the way the candidate completions are
returned from cmd_getlist. All fixes I can see involve major
surgery, so here I tend to pull an "I'd rather not".

Would leaving (2) open be acceptable?

Edit (2015-04-14): (1) is worse than I'd have expected. I don't think there
is generic code for locale-independent parsing for what ctime() returns
commonly around. This means that what xombrero currently uses in its history
file is fairly hard to parse without fixing the locale (as in: is currently
broken when a history file is transported from one locale to another). With
this patch, that then also concerns favourites. There's
parseDateFromNullTerminatedCharacters in webkit's DateMath.cpp that prabably
would do the trick, but that doesn't look like something one would like to
depend on.

The only good solution occurring to me is changing the on-disk format to store
dates either as time_t somehow shoehorned into an integer or float or
store it in YYYY-MM-DDThh-mm-ss ISO format that, as long as one doesn't have
fractional seconds, doesn't have locale issues I'm aware of with strptime.
The current parsing code would be tried first so the files would be
transparently migrated.

Easier, but hackier, would be to set LC_TIME; but that'd break history files
written in non-C locales.

So -- I'm happy to implement either time_t or ISO time storage for
both favourites and history. Does anyone have preferences?

@msdemlei msdemlei mentioned this pull request Apr 19, 2015
@msdemlei
Copy link
Author

Superseded by #91

@msdemlei msdemlei closed this Apr 19, 2015
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.

2 participants