search

Evpok / latex-autocomplete

LaTeX autocompletion for atom
Other
9 stars 3 forks source link

Support other LaTeX grammar providers #26

Closed Aerijo closed 6 years ago

Aerijo commented 6 years ago

The original checked if the grammar name equalled LaTeX, which does not follow any sort of regulation and cannot be expected to be true for all LaTeX language providers.

The proposed change replaces the check with scopeName, which is far more consistent across language packages; if a grammar is for LaTeX, the scope name will be text.tex.latex. I also changed the equality to ===, as the checking is between two strings and seems more appropriate.

Aerijo commented 6 years ago

We're also considering changing the scopeName value to source.latex some point in the future, so I'll come back if we do.

Evpok commented 6 years ago

Looks good to me. Have you tested it against language-latex ?

Aerijo commented 6 years ago

Yes, the package activates as expected when working on a LaTeX file (either inferred by file extension or manually set). It works on language-latex and also my own one language-latex2e (which is why I made this PR to begin with; it wasn't recognising when alternative grammar providers were used).

I just noticed a bug when testing this now; if the file is not saved to disk (so just a new file in Atom) and the grammar is manually set to LaTeX it will complain with an error saying 

Uncaught TypeError: Cannot read property 'path' of null

/Users/benjamingray/.atom/packages/latex-autocomplete/lib/macros_autocompletion/macro_completer.js:31
Hide Stack Trace
TypeError: Cannot read property 'path' of null
    at Object.getSuggestions (/Users/benjamingray/.atom/packages/latex-autocomplete/lib/macros_autocompletion/macro_completer.js:31:82)
    at providers.forEach.provider (/Applications/Atom.app/Contents/Resources/app/node_modules/autocomplete-plus/lib/autocomplete-manager.js:286:58)
    at Array.forEach (native)
    at AutocompleteManager.getSuggestionsFromProviders (/Applications/Atom.app/Contents/Resources/app/node_modules/autocomplete-plus/lib/autocomplete-manager.js:262:21)
    at AutocompleteManager.findSuggestions (/Applications/Atom.app/Contents/Resources/app/node_modules/autocomplete-plus/lib/autocomplete-manager.js:254:23)
Evpok commented 6 years ago

Yes, seeing this too, so it's unrelated to your PR (good catch, though !).

Anyway, seems we are good to merge, thanks for the contribution !