search

Trevoke / sqlup-mode.el

An emacs minor mode to upcase SQL keyword and functions
GNU General Public License v3.0
88 stars 15 forks source link

Feature no global evals #46

Closed yogsothoth closed 8 years ago

yogsothoth commented 8 years ago

This implements upcasing for eval strings without relying on global variables. The new implementation also works with multi-line eval strings.

I also added some test cases with ecukes. I could not find a way to check a multi-line string, though.

Note that the branch was derived from add-testing.

Trevoke commented 8 years ago

Awesome. I'll merge this in the next two days, most likely. Steve Purcell just confirmed that directories aren't pulled in by default, which means we're good : https://github.com/melpa/melpa/issues/3954 and this means that I was able to merge the add-test branch!

Trevoke commented 8 years ago

I'm not explaining myself well. Yes, you have that test. What I am saying is that the test I showed you ALSO passes, and I'm asking if that's a problem.

And if it is a problem, I'm asking if we care or if it's OK for now.

yogsothoth commented 8 years ago

Ah, alright, I get it now. Sorry I thought this was all about an issue with merging the tests.

Having only format isn't correct, so I guess in sqlup-mode.el we should change :

(defconst sqlup-eval-keywords '((postgres "EXECUTE" "format(")) "List of keywords introducing eval strings, organised by dialect.")

To something like

(defconst sqlup-eval-keywords '((postgres "EXECUTE" "EXECUTE format(")) "List of syntax elements introducing eval strings, organised by dialect.")

Good catch and apologies for the confusion, my brains must be on holiday as well...

yogsothoth commented 8 years ago

I just remembered why the current implementation has only "format (" and not "EXECUTE format(": this is to allow for an arbitrary number of spaces and newlines to take place between EXECUTE and format, as I didn't want to clutter the keyword list with regexp tokens. The side effect of course is that an isolated format is seen as starting an eval string, even as it isn't syntactically valid.

Actually, I would tend to think that allowing SQL developers to format their code any way they want is more important than refusing to upcase keywords after faulty syntax: this will be caught by the interpreter. After all, we do upcase things like "select case where 1", even though it isn't valid SQL at all.

So I would say that we can leave things as they are. What's your opinion on this?

Trevoke commented 8 years ago

I think that leaving things as they are for now is fine; if we need to make further modifications, we can switch string-equal to string-match and do a regexp comparison instead.

Trevoke commented 8 years ago

Got the time to merge it in! Thanks so much for all your help so far :)