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

Define a formatter for SQL (sqlformat) #11

Merged
merged 3 commits into from Jul 10, 2018

Conversation

Projects
None yet
2 participants
@knu
Copy link
Collaborator

knu commented Jul 2, 2018

It uses sqlformat from the Python package "sqlparse" with some options of choice:

  • -k upper: to uppercase keywords
  • -a: to get a multiline, aligned output
@lassik

This comment has been minimized.

Copy link
Owner

lassik commented Jul 2, 2018

Hey, I remember you from FreeBSD from many years ago! :D Great to see that you're still coding. Thanks for contributing :)

I quickly tested that it works. However, I immediately hit a bug with the formatter itself (sqlparse issue #313, a fix should be coming to pip with version 0.3.0).

The installation command seems to be just:

pip install sqlparse

without a 'py-' prefix.

The FreeBSD port seems to have a 'py-' prefix, as is conventional there. In principle, it's possible to specify a separate install command for FreeBSD, but we currently choose the operating system based on the Emacs system-type variable. It seems system-type only has a generic berkeley-unix category. We should make our own variable that differentiates between Linux/Darwin/FreeBSD/OpenBSD/NetBSD/Windows. I'll try to get it done this week (issue #12).

We could also add

"--encoding"
(coding-system-change-eol-conversion buffer-file-coding-system nil)

to the command line arguments.

@knu knu force-pushed the knu:sqlformat branch from f87c968 to 1d5d183 Jul 2, 2018

@knu

This comment has been minimized.

Copy link
Collaborator Author

knu commented Jul 2, 2018

Oops, I've removed the superfluous py- prefix.

As for encoding, despite this PR merged, the sqlformat command does not seem to handle UTF-8 well with Python2, so I recommend you use Python3 for now; sqlformat does not (yet) have an --encoding option.

And yes, I still use FreeBSD on my personal server and make commits from time to time. 😁 I code a lot mainly in Ruby/Rails and ECMAScript in the web development scene, although Emacs is no longer very popular there. 😆

@lassik

This comment has been minimized.

Copy link
Owner

lassik commented Jul 2, 2018

Thanks for updating the code.

If Python 2 is known to be problematic, we could just change the install command to pip3 install sqlparse (note the 3 in pip3). Python 3 / PIP 3 ought to be available everywhere nowadays.

I just added a freebsd system type for the install commands :) https://github.com/lassik/emacs-format-all-the-code/blob/master/format-all.el#L72

I installed the latest sqlparse from pip today, and that version of sqlformat does have an --encoding option:

$ sqlformat --version
0.2.4
$ sqlformat --help | grep -i encoding
  --encoding ENCODING   Specify the input encoding (default utf-8)
$ echo ååöÅÄÖ | iconv -f utf-8 -t iso8859-1 | sqlformat --encoding iso-8859-1 -
ååöÅÄÖ

Yeah, unfortunately Emacs' GUI framework has needed modernization for 20 years. And font-lock should finally be replaced with a real parsing framework with multi-language support. Young web devs will no longer put up with these things. I hope our venerable editor survives the next decade. MELPA offers a glimmer of hope :)

@lassik

This comment has been minimized.

Copy link
Owner

lassik commented Jul 7, 2018

@knu I'm ready to merge this, but what do you think we should do about the pip3 suggestion and the --encoding option in light of my comments above?

@knu

This comment has been minimized.

Copy link
Collaborator Author

knu commented Jul 8, 2018

I'm not sure if pip3 is a widely available name for pip for Python3. I guess python3 -m pip would be a better bet, but in that case, you'd have to call sqlformat as python3 -m sqlparse. Maybe we should work out a way to use Python2 for now...

@lassik

This comment has been minimized.

Copy link
Owner

lassik commented Jul 8, 2018

If the only problem with Python 2 is the encoding, we could just make it pip (which could be either Python 2 or Python 3, depending on the OS) and leave out the --encoding option for now.

@knu

This comment has been minimized.

Copy link
Collaborator Author

knu commented Jul 9, 2018

I've finally got it to work with Python 2.7. I had to set PYTHONIOENCODING for Python <3.2 including 2.7 because the older versions of python wouldn't respect the locale nor default the encoding to UTF-8 when the output is redirected, only to fail in trying to output in ASCII encoding.

@lassik

This comment has been minimized.

Copy link
Owner

lassik commented Jul 9, 2018

That's awesome, thank you for the effort :)

Are you sure ienc and oenc are the right way around? The docstring for default-process-coding-system says:

The car part is used for decoding a process output,
the cdr part is used for encoding a text to be sent to a process.

which seems like they are opposite to the intuitive order. Also, sqlformat --help says:

--encoding ENCODING Specify the input encoding (default utf-8)

So that's input too. But what is the output encoding, is it always the same as the input encoding, or does it come from PYTHONIOENCODING even if --encoding is given?

Sorry about all this detail...

@lassik

This comment has been minimized.

Copy link
Owner

lassik commented Jul 9, 2018

Also, we should probably set envars like:

(let ((process-environment (cons (concat "PYTHONIOENCODING=" ienc)
                                 process-environment)))
  (format-all-buffer-process executable ...))

instead of depending on the external env program.

@knu knu force-pushed the knu:sqlformat branch from ff197a8 to 3d35822 Jul 10, 2018

@knu

This comment has been minimized.

Copy link
Collaborator Author

knu commented Jul 10, 2018

Good points, fixed!

@lassik

This comment has been minimized.

Copy link
Owner

lassik commented Jul 10, 2018

Looks good to me. Finally merging :) Thanks for your contribution!

@lassik lassik closed this Jul 10, 2018

@lassik lassik reopened this Jul 10, 2018

@lassik lassik merged commit 898c543 into lassik:master Jul 10, 2018

@knu knu deleted the knu:sqlformat branch Jul 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.