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

ref/cite completion: fixes and enhancements #120

Merged
merged 25 commits into from Jun 29, 2013

Conversation

Westacular
Copy link
Contributor

I've made a bunch of refinements/refactoring to the ref and cite completion systems:

  • For each of ref and cite, I've refactored the common code between the autocomplete plugin and the quick panel command into a single shared function.
  • Fixed an issue where the commands would crash if the current file is unnamed and unsaved.
  • Fixed various cases where the completion would generate an extra leading backslash, or a missing closing brace, etc.
  • Changed the ref/cite dispatching command so that it directly uses the actual regexes used internally in the ref and cite functions to determine which, if any, to use. (Previously the dispatcher function was too restrictive, and would reject otherwise valid usage scenarios.)
  • Make the autocomplete plugins less eager (see [Issue 118](see: automatic expansion of 'ref' #118)), so that they don't make unwanted modifications to the text if the user is in the middle of typing something else.
  • Enhanced cite completion to work with multiple, comma-separated references in a single \cite{} command. (I didn't do this for ref, since most ref commands do not work with a list of multiple references.)
  • Changed ref completion so that it will also work when other known ref commands are used (e.g. pageref, autoref, cref, etc.).
  • Added keybinding for "{" and "," such that, if they are typed in very specifically appropriate contexts (e.g., "{" following "\ref"), they will automatically open the appropriate completion menu. This feature can be disabled by adding "disable_latex_ref_cite_auto_trigger": true to your user preferences.
  • Incorporated jlegewie's options for customizing the format of the citation quick panel.

…e ref/cite dispatcher more tolerant of allowed formats
…context is appropriate

i.e., if the cursor is immediately preceded by "\ref" or "\cite" or recognized variants.
… function. Also, fixed a few corner cases along the way.
…s (shouldn't actually change what is/isn't matched)
…ept more of the forms that the completion functions internally accept.
…es as used in the ref and cite completion functions.
…quick panel from working except at the start of a line.
…et to true, stops { and , from automatically opening the ref/cite completion quick panels.
@msiniscalchi
Copy link
Collaborator

THis looks amazing! I really have very little time these days, but this
looks like something that could go in.

My only issue is that I prefer small, atomic commits rather than one big
code drop. But, I'll take what I can get :)

On Mon, Dec 10, 2012 at 4:02 PM, Wes Campaigne notifications@github.comwrote:

I've made a bunch of refinements/refactoring to the ref and cite
completion systems:

  • For each of ref and cite, I've refactored the common code between
    the autocomplete plugin and the quick panel command into a single shared
    function.
  • Fixed an issue where the commands would crash if the current file is
    unnamed and unsaved.
  • Fixed various cases where the completion would generate an extra
    leading backslash, or a missing closing brace, etc.
  • Changed the ref/cite dispatching command so that it directly uses
    the actual regexes used internally in the ref and cite functions to
    determine which, if any, to use. (Previously the dispatcher function was
    too restrictive, and would reject otherwise valid usage scenarios.)
  • Make the autocomplete plugins less eager (see Issue 118), so that
    they don't make unwanted modifications to the text if the user is in the
    middle of typing something else.
  • Enhanced cite completion to work with multiple, comma-separated
    references in a single \cite{} command. (I didn't do this for ref, since
    most ref commands do not work with a list of multiple references.)
  • Changed ref completion so that it will also work when other known
    ref commands are used (e.g. pageref, autoref, cref, etc.).
  • Added keybinding for "{" and "," such that, if they are typed in
    very specifically appropriate contexts (e.g., "{" following "\ref"), they
    will automatically open the appropriate completion menu. This feature can
    be disabled by adding "disable_latex_ref_cite_auto_trigger": true to
    your user preferences.
  • Incorporated jlegewie's optionshttps://github.com/SublimeText/LaTeXTools/pull/92for customizing the format of the citation quick panel.

You can merge this Pull Request by running:

git pull https://github.com/Westacular/LaTeXTools autocomplete

Or view, comment on, or merge it at:

#120
Commit Summary

  • ref and cite autocomplete would crash if the current file is unnamed
  • Correct bug that could result in "\ref{" (note double slash) and
    mak…
  • Automatically open the ref/cite completion list when typing { if the
  • Make command versions of cite/ref autocompleters also tolerant of
    the…
  • Refactor common code of the autocomplete plugin and the quick panel
    c…
  • Rename error type, remove overzealous error response.
  • Refactor common autocomplete / quick panel command code into a
    shared…
  • Tweak ref_cite regexes to more accurately align with new and old
    form…
  • Enable completion of multiple, comma-separated references in a
    single…
  • Correct some typos
  • Make ref and cite autocompletion slightly less eager (see:
    https://gi… automatic expansion of 'ref' #118
  • Some more fixes to the ref/cite completion dispatcher regexes, to
    acc…
  • Some fixing of the fix to make autocomplete less disruptive to
    normal…
  • Set the ref/cite dispatcher to just directly use the exact same
    regex…
  • Fix an error that prevented the { and , auto-opening of the cite/ref
  • Add to ref completion recognition of/support for a variety of other
    r…
  • Add a setting -- "disable_latex_ref_cite_auto_trigger" -- that when
    s…
  • Incorporating jlegewie's options for formatting of citation quick
    panel
  • Fix an error with ref completions.
  • Tweak the { and , completion triggers, and add them to Linux and
    Wind…

File Changes

  • M Default (Linux).sublime-keymap (18)
  • M Default (OSX).sublime-keymap (18)
  • M Default (Windows).sublime-keymap (18)
  • M LaTeXTools Preferences.sublime-settings (22)
  • M getTeXRoot.py (32)
  • M latex_cite_completions.py (548)
  • M latex_ref_cite_completions.py (10)
  • M latex_ref_completions.py (267)

Patch Links

Marciano Siniscalchi
Economics Department, Northwestern University
http://faculty.wcas.northwestern.edu/~msi661

@Westacular
Copy link
Contributor Author

Thanks! And, yeah, sorry about the size of the change. Almost all of the individual commits are pretty small, but I got on a roll, and they started to stack up.

Also, there's no real way to stop a refactoring of the guts of the completion into shared functions (f0ef951 and c544ab8) from showing up as a huge diff, even though most of the code is actually unchanged in both those commits.

@rmgk
Copy link

rmgk commented Dec 10, 2012

i tested if this resolves Issue #118 and found that it indeed does,
but the auto completion trigger on '{' does not seem to work for me,
which makes me unable to insert '{' after '\ref' unless i deactivate that keybinding.
as a note, even without these changes there is no autocomplete popup when i type 'ref' and it gets expanded.
console says that it parsed the file, but no popup, pressing ctrl+space (in a '\ref{}') does open a popup with correct completions.

also with these patches everything i type is echoed back in the console

@Westacular
Copy link
Contributor Author

Thanks for the feedback!

Regarding the echoing: there's a lot of diagnostic print statements in there. I haven't added any, but I didn't turn any off, either. (They should probably be turned off eventually, though.)

It's odd that the '{' trigger is failing. What you're describing implies that the context is matching enough to activate the keybinding (and thus run the completion function), but something in the function must be failing (or hitting an exception) before it gets to the point where a { would be inserted -- and doing so in a way that doesn't generate an error message. I'm not sure what could be causing that. What gets output on the console when this happens?

@rmgk
Copy link

rmgk commented Dec 11, 2012

i have compared what happens on my windows desktop (which is where the error occurs) with my linux laptop.
basically, without any the changes here, the autocompletion does not work correctly on my windows machine.
on linux i do get a popup when completing 'ref' on windows i do not.
the output when that happens is on both machines:

TEX root: u'/path/to/file.tex'
Searching file: u'/path/to/file.tex'
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 7: ordinal not in range(128)

i also tried with a file which does not contain any unicode characters.
the same happens, but without the UnicodeDecodeError.

so basically there seems to be some other error that prevents the command from completing,
thus preventing the character to be inserted.

i will later try with a clean version of sublime, when im back on my desktop.

…statements)

The Sublime terminal apparently only handles ASCII, so printing unicode text to it causes exceptions. So I've disabled several superfluous print statements that can run into trouble there.

Also, the ref completion would hit an exception if it read a \label from a file that contained a non-ASCII character. I've added code to check for \usepackage[.*]{inputenc} and, if present, read the file using the appropriate decoder.

Similarly, I've added analogous code to the cite completion, although in that case it's only guarding against non-ASCII characters in .bib filenames; the contents of the .bib files are still assumed to be ASCII. This is a tricky problem since the character encoding of .bib files is a messy topic. (And since the bib parsing needs other fixes anyway, it's probably not worth worrying about it too much right now.)
@Westacular
Copy link
Contributor Author

I went through and made some changes that I hope fix a couple issues regarding the handling of unicode characters. This might solve the '{' not appearing issue, but I'm not sure; from what you're saying, there could still be another problem.

… the completion crashes or fails miserably, at least a { or , should be inserted.
@Westacular
Copy link
Contributor Author

This latest commit should solve the problem of { not appearing, even if I've still missed some other underlying issue that's stopping the completion menu from working.

@rmgk
Copy link

rmgk commented Dec 12, 2012

i tested it again with the current version and everything seems to work for me now.
although i do not like the workflow of switching to the command palette selection when typing '{',
this is not a problem for me because disabling the keybinding just gives me the expected completions when i continue typing.

so i say my thanks.
merging this will close issue #118 as far as im concerned.

@Westacular
Copy link
Contributor Author

Yeah, the cite regexes had no awareness of optional arguments to the cite command. 404c0bf fixes that. Thanks for pointing it out.

@jlegewie
Copy link

jlegewie commented Jan 8, 2013

It works great with the commit as long as there is only one citation in the paragraph. If there are any other citations before the one I want to add, I get the Ref/cite: unrecognized format error. Can you reproduce that?

@Westacular
Copy link
Contributor Author

No, everything seems to be working for me. Could you post a text sample for where it's failing?

@jlegewie
Copy link

jlegewie commented Jan 8, 2013

Yes, you are right. It was a problem with one of my custom key combinations. I have one that opens the quick select panel when I press tab and the cursor is inside the brackets of \citep{}, which seems to cause the problem. Sorry for the trouble. I have been using your autocomplete branch for a while now and it works smoothly. I hope it gets merged soon.

@msiniscalchi
Copy link
Collaborator

THhis is all amazing work guys. I promise I'll pull this one way or
another, as soon as I have time.

On Tue, Jan 8, 2013 at 3:13 PM, jlegewie notifications@github.com wrote:

Yes, you are right. It was a problem with one of my custom key
combinations. I have one that opens the quick select panel when I press
tab and the cursor is inside the brackets of \citep{}, which seems to
cause the problem. Sorry for the trouble. I have been using your
autocomplete branch for a while now and it works smoothly. I hope it gets
merged soon.


Reply to this email directly or view it on GitHubhttps://github.com//pull/120#issuecomment-12017548.

Marciano Siniscalchi
Economics Department, Northwestern University
http://faculty.wcas.northwestern.edu/~msi661

@stared
Copy link

stared commented Jan 22, 2013

Does is also solve Issue 134?
(That is, does it parse entries separately, allowing that "author" may not exist?)

I writing my own script to fix that (and also show journal name in the search panel), but it does not use regexes (to avoid problems with nested {/}).

It goes like that (just functions):

entries = data.split("@")

def process_entry(entry):
    level = 0
    last_split = 0
    splitted = []
    for i, l in enumerate(entry):
        if l == "{":
            level += 1
            if level == 1:
                last_split = i + 1
        elif l == "}":
            level -= 1
            if level == 0:
                splitted.append(entry[last_split:i].strip())
                break
        elif level == 1 and l in [",", "="]:
            splitted.append(entry[last_split:i].strip())
            last_split = i + 1
    return splitted


def extract_fields(splitted, fields):
    pairs = []
    for i, splinter in enumerate(splitted):
        if splinter in fields:
            pairs.append((splinter, bib_strip(splitted[i + 1])))
    return dict(pairs)


def bib_strip(s):
    if s[0] == "{" and s[-1] == "}":
        return bib_strip(s[1:-1].strip())
    else:
        return s

@Westacular
Copy link
Contributor Author

No, it doesn't really fix the problem. It's still using the quick and dirty regexes to pull out the field values. It DOES avoid the problem of having the wrong author shown, but that's because in the case where there's an entry with no author field, it returns an error and aborts, instead of continuing with mismatched values.

Your approach is a much better idea, but there was talking of adding a proper full bib parser, so I didn't do much to change the current parsing code in my patches.

@stared
Copy link

stared commented Jan 22, 2013

Good to hear that is fixes "wrong author" problem. :)

I wanted to fix it with regex, about observed that it cannot work in general (just, typically, fields does not contain ,\nauthor = { blah blah }; but in principle they could).

The "clean" solution is just above + actually running it:

results = []  # i.e. the first and second line in search palette
for entry in entries[1:]:
    splitted = process_entry(entry)
    bibkey = splitted[0]
    di = extract_fields(splitted, ["title", "author", "journal", "year"])
    line1 = "%s (%s)" % (di["title"], bibkey)
    try:
        line2 = "%s\t\t" % di["author"]
    except:
        line2 = "(no author provided)"
    try:
        line2 += "in %s " % di["journal"]
    except:
        pass
    try:
        line2 += "(%s)" % di["year"]
    except:
        pass
    results.append((line1, line2))

However, as it is going to touch the same lines (and Pull request - Response time is non-trivial), I don't want to mess up with your commits (fixing a lot of things).

And anyway, we don't need full BibTeX parsing. E.g. I tried https://github.com/ptigas/bibpy but seems to be too slow for our purpose + messes with {/} anyway (but the good thing is that it also extracts author names and surnames).

@carlton87
Copy link

Could you add support for other cross-ref­er­enc­ing commands like \cref (from the cleveref package), \vref (from the varioref package), or \autoref? I know it would be possible to map one of them to \ref via the \let command from within LaTeX but sometimes a distinction is necessary.

@Westacular
Copy link
Contributor Author

Actually, this branch already does add support for a variety of other cross-referencing commands, including all of the ones you mention.

Specifically, it will recognize and autocomplete with any of:

\pageref
\vref
\Vref
\autoref
\nameref
\cref
\Cref
\cpageref

Let me know if there's any I missed.

@escherpf
Copy link

This is an awesome addition. Thank you for your work on this.

One very minor observation. I find that the "{" trigger would work better with the "," trigger if, after making a selection from the quick view panel following the "{" trigger, the caret stayed inside the closing bracket, instead of ending up outside of the closing bracket as it currently does. Thus, adding multiple citations requires one to backspace to the end of last citation keyword in order to activate the "," trigger.

Admittedly, people who do not often use multiple citations may not find this behavior desirable. Given that I use a keybinding specifically to step outside of brackets, however, I find it much easier to skip over a closing bracket if I only want a single citation than to jump back to activate the "," trigger if I want multiple citations.

@MattDMo
Copy link
Member

MattDMo commented May 17, 2013

Has this been merged into a branch, yet? I was just working on a StackOverflow question where the user had "auto_complete_selector": "source, text" and was trying to type the word "reform" but kept getting \ref{ auto-completed whenever "ref" was typed. S/He otherwise liked the auto-complete features and didn't want to disable it. I didn't have time to search the Issues this morning when I answered him, and my only suggested workaround was to completely disable latex_ref_completions.py by renaming it, deleting the .pyc file, and restarting Sublime. If I just change the contents of that file to what's in this commit, will that solve the problem, or are there other things that need to be fixed? Hopefully this can be committed soon! (wink wink nudge nudge)

@msiniscalchi
Copy link
Collaborator

@Westacular just a quick heads-up: I'm now finally able to work on the plugin some more!

Merging this will be a lot of work, BUT it's actually something I'd very very very much like to use myself! I hope to find some quiet time to do it soon. I may have questions for you. And THANKS!!!

@msiniscalchi
Copy link
Collaborator

Let me also thank @jlegewie and @yunjing here, and also make a note so I don't forget to thank them in the README once I pull this in.

@msiniscalchi
Copy link
Collaborator

OK, I couldn't resist and started playing with it :)

Right now I'm having the problem that my bibliography (which has grown "organically" over the years, i.e., it's got all kinds of crap, even though it works fine in both bibtex and LaTeXtools) is not parsed correctly. Too bad! I'll look into it.

@msiniscalchi
Copy link
Collaborator

Ok @Westacular and @jlegewie , if you check the Westacular-autocomplete branch, you will see my first stab at merging this. The key thing is that my crufty bib file brought up issues with the way we parse bibliographies (you inherited this from my code, so it's my fault...).

I have now modified the code in latex_cite_completion.py so as to parse the bib file differently. Basically, as I read the file, I keep track of matches for title, author, year and editor (in case there is no author---common), and then, as soon as I get an "@" with a keyword, I append the current values of title, etc. onto the corresponding arrays (titles, etc.). This way we are guaranteed to always have complete records, or at least placeholder text for missing entries.

I also had to fix a couple of minor issues with the year and author regexes.

Bottom line: I'd like to experiment with this some more. For instance, the parsing code is now a bit slow (on a slow machine) because it is an explicit iteration. More importantly, I want to use it in my own day-to-day work to see how robust it is. If it passes this "test", I'll merge it into the master branch.

Guys, THANK YOU for this AWESOME contribution! You scratched many of my own itches, and the functionality is outstanding!

@jlegewie
Copy link

Great that you are making progress! Westacular deserves basically all the credit...

Which branch are you using now. I might change to it and test as well. Here are some quick notes:

  • We had some discussion about the default. I am not sure what Westacular's current pull request does but my original one left your default in tact. You might still want to consider changing the default. I am using ["{author_short} {year} - {title_short} ({keyword})","{title}"]
  • I guess the best way would be to use a real bibtex parser. But that is a more work and probably not the right solution for now. Just something to keep in mind.
  • One possible way to resolve the performance issue would be to store the parsing results and only update them when the bibtex file has changed. Each time the user wants to add a citation, LaTexTools would check whether the bibtex file has changed and if not, just use the references in memory. If the file has changed LatexTools could re-process the file.

But that are just some thoughts...

@msiniscalchi
Copy link
Collaborator

Hi @jlegewie , I followed github's instructions for non-automatically-mergeable patches. I created a local branch on my machine and pulled from

https://github.com/Westacular/LaTeXTools.git

This was two days ago. Then, I had to integrate the code with some fixes that had already gone into the master branch, etc.---that was all pretty straightforward. All the work (late-evening hacking session) went into the new bibtex parsing code.

To see what I've done, get the Westacular-autocomplete branch of the "official" LaTeXTools repository. I'll be working on that one, and if you want to contribute patches, they should go against that. Again, I think it's pretty much in sync with @Westacular 's own fork.

Re. speed, I also thought about caching, but we'll see. It turns out maybe I was too pessimistic. I only noticed a slight delay after e.g. entering \ref{ on my Samsung 7 Slate (pre-Win8 tablet) when in "Power Saving" mode (which is very aggressive), with my old, crufty, and 9401-lines long bib file. On my 2008 Macbook Pro, there is no delay. Even on the Windows tablet it's not so bad. In any case, in the next few weeks the Windows tablet will be my only machine, so I have every incentive to make things work as smoothly as possible!

Re. parser, sure, a complete parser would be nice to have, but not essential ATM. Right now we need to get keyword, title, author and year only. That's easy to do with ad-hoc code. As you will see, the code is pretty easy to read, and it also does the right thing (the old code was a kludge---each line was parsed multiple times, for instance, even in case of a match).

Right now the default is the "fancy" one you guys put in, which I love and which will make several users happy! Not to mention the fact that the multi-cite support totally ROCKS!

Again, I want to "battle test" this for a couple of days, do a few tweaks, and then merge with the master branch. But, feel free to make suggestions, etc. Do please take a look at my code in the Westacular-autocomplete branch.

THANKS!

@stared
Copy link

stared commented Jun 24, 2013

@msiniscalchi We are happy to have you back.

For me autocomplete from @Westacular works great, even for >1MB bib file with lot of strange things (like LaTeX codes for non-latin characters in authors).

I added a few changes to that branch: #207.
I am curious if you (and @Westacular) like it.

@msiniscalchi
Copy link
Collaborator

How is performance from my branch with the westacular improvements?

@stared
Copy link

stared commented Jun 24, 2013

As I said, for me (Mac Book, 2009) it works smoothly on 1MB bib file. But I don't know how it works for others.

@stared
Copy link

stared commented Jun 24, 2013

...and as a side remark "no bib found" is very annoying when bibliography is in the file (as e.g. it is the almost-final version before sending to a journal or posting on arXiv).

@msiniscalchi
Copy link
Collaborator

@stared processing in-file bibliographies is going to require separate code, and won't be as neat. The reason is that each \bibitem entry does not follow a pre-specified format, so it's hard (read: impossible) to guess correctly what the author, title, journal, etc. are. However, I guess one can at least list the keywords.

@Westacular @jlegewie I've tweaked the code a bit more and have been testing it. If you can also test it, and no issue arises, I'll merge!

@stared
Copy link

stared commented Jun 26, 2013

@msiniscalchi I know that the processing is different, so my point is not to implement it (at least for my workflow once bibliography is hardcoded into the tex file, I don't need that much to search it). However, it is ultra-annoying when every time I write \cite{ it raises an alert No bib files found!.

I am thinking what is the best way around (I don't want to silence it in every case - silenced errors are annoying). Any ideas? Maybe error msg only when a shortcut is used?

@msiniscalchi
Copy link
Collaborator

@stared got it. Now, there is an option to turn of all autocite/autoref completions. I added the following

  • separate preferences to automatically fire ref and cite completions: cite_auto_trigger and ref_auto_trigger. By default completion is triggered, but you can change this setting in the usual LaTeXTools Preferences file
  • probably more useful to you: you can also toggle autocompletion on and off from the keyboard, without messing with config files. The keybindings are c-l,t,a,c and c-l,t,a,c (where c-l as usual stands for control or command as the case may be). If you type c-l,t,?, the current toggle and pref settings are displayed in the status bar.

This should get rid of your problem. @Westacular and @jlegewie I am thinking about removing the "disable_ref_cite_auto_trigger" setting that you allow for in the keybinding, as the above subsumes it.

@stared
Copy link

stared commented Jun 28, 2013

@Westacular @msiniscalchi Just now I saw than for some reason wrapping in environment (["super+l","super+n"], and other commands suing super two times) does not work in the Westacular-autocomplete branch. Are you experiencing the same thing?

@msiniscalchi
Copy link
Collaborator

Can someone else confirm? Everything seems OK here (on Windows;
unfortunately I won't be able to test on Mac for a month or so).

On Fri, Jun 28, 2013 at 3:45 PM, Piotr Migdał notifications@github.comwrote:

@Westacular https://github.com/Westacular @msiniscalchihttps://github.com/msiniscalchiJust now I saw than for some reason wrapping in environment (
["super+l","super+n"], and other commands suing super two times) does not
work in the Westacular-autocomplete branch. Are you experiencing the same
thing?


Reply to this email directly or view it on GitHubhttps://github.com//pull/120#issuecomment-20213261
.

Marciano Siniscalchi
Economics Department, Northwestern University
http://faculty.wcas.northwestern.edu/~msi661

@msiniscalchi msiniscalchi merged commit 404c0bf into SublimeText:master Jun 29, 2013
@stared
Copy link

stared commented Jul 2, 2013

@msiniscalchi At least on the main branch everything works (i.e. both bibliography and double super). So it's fine, I am happy (and thankful).

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

8 participants