Skip to content

Conversation

bsmithgall
Copy link

Cool project!

This fixes #136 using the suggested sql formatter library. I included the minified version in the bundle and added the relevant scripts to the codemirror includes instead of adding new files, though I could also add new files. I wanted to keep it all together, since the result of the format needs access to the editor in order to properly update the codemirror instance.

@simonw
Copy link
Owner

simonw commented Apr 3, 2018

Here's what this looks like:

2018-04-03 at 8 32 am

I need to figure out the right way to handle licensing of bundled software like this - it's MIT licensed which is compatible with Datasette's Apache 2 license, but I feel like bundled licensed software (including codemirror) needs to be recognized in the README or docs somehow.

@simonw
Copy link
Owner

simonw commented Apr 3, 2018

Let's only show the "Format SQL" button if the user has JavaScript enabled. We can do that in this code here:

https://github.com/bsmithgall/datasette/blob/4a7151a58d6ab7c8404a91beef7083e8a5807cf8/datasette/templates/_codemirror_foot.html#L14-L21

@simonw
Copy link
Owner

simonw commented Apr 3, 2018

On the licensing front: it looks like the way Django handles this is to keep the licensing header in the files intact, e.g. https://github.com/django/django/blob/6deaddcca367d0143c815aaa42342021baa3b41e/django/contrib/admin/static/admin/js/vendor/jquery/jquery.js

So for this change, adding a comment at the top of sql-formatter.min.js which references the MIT license would do the trick.

@bsmithgall
Copy link
Author

I can work on that -- would you prefer to inline a display: hidden and then have the javascript flip the visibility or include it as css?

@simonw
Copy link
Owner

simonw commented Apr 9, 2018

I'd prefer to have the JavaScript actually manipulate the DOM to add the button - something like this:

var button = document.createElement('button');
button.value = 'Format SQL';
button.addEventListener(
    'click', format, false
);
document.getElementById('run-sql').parentNode.appendChild(button);

@bsmithgall
Copy link
Author

I've implemented that approach in 86ac746. It does cause the button to pop in only after Codemirror is finished rendering which is a bit awkward.

@simonw
Copy link
Owner

simonw commented Nov 11, 2019

Closing this because this feature was shipped in #592

@simonw simonw closed this Nov 11, 2019
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.

"Reformat SQL" button next to SQL editor textarea
2 participants