alex-hhh / emacs-sql-indent

Syntax based indentation for SQL files inside GNU Emacs
GNU General Public License v3.0
121 stars 18 forks source link

org-mode source blocks? #96

Closed jaidetree closed 4 years ago

jaidetree commented 4 years ago

Any insight into how one can have sqlind-minor-mode inside of org-mode src sql blocks?

#+BEGIN_SRC sql
SELECT
    id,
    name
  FROM contacts
#+END_SRC

Say you highlight the text in the src block and try to indent it with TAB. I would expect it not to change (or slightly if my formatting is off) but instead it all left aligns. If I do the popout editor and do the same thing, sqlind minor mode is active and it indents correctly.

PierreTechoueyres commented 4 years ago

Only for that block ? I don't know ! But if you enable sqlind-minor-mode for buffers with sql-mode then I think you could achieve that. You could do that using the sql-mode-hook with something (untested) like bellow:

(add-hook 'sql-mode-hook '(lambda () (sqlind-minor-mode 1)))

But If you're using emacs 27.1 and have installed this package then it should already be enabled ...

alex-hhh commented 4 years ago

I am not using source blocks in org mode, but a quick check, indentation does not seem to work with the C or Pyhton languages either. So perhaps, this is not supported in org-mode?

I would suggest you ask this questions in org-mode discussion lists for help about this.

jaidetree commented 4 years ago

Thanks for the support!

I was enabling the minor mode in the sql-mode-hook already 🙂

(use-package! sql-indent
  :hook ((sql-mode-hook . sqlind-minor-mode)))

@alex-hhh You're probably right, but can you please check if you have these settings:

org-src-tab-acts-natively     t
org-src-preserve-indentation  t
org-src-fontify-natively      t

Supposedly that allows langs to dictate the indentation which I'd like to confirm to make sure it's an org issue and not more isolated.

alex-hhh commented 4 years ago

You are correct, I did not have these settings (I told you I don't use that feature in org mode). However, if I enable them, the SQL mode indentation now works (along with the indentation for C and Python blocks). I didn't do anything special with my setup, it just worked once I enabled the org mode features.

jaidetree commented 4 years ago

Thanks for looking into it! The fact that it's working for you means this is likely a problem I caused in my config.

I'm pretty intermediate with emacs but I won't waste your time with too much backstory, I was inspired by https://youtu.be/dljNabciEGg to get more out of org-mode. I have to go through 300 tables to truncate\prune what I can for a lighter development DB import. Being able to document each table's purpose and record the queries for inspection\truncating that are executable from within the org-mode src block has proven an extremely pleasant approach so far.

alex-hhh commented 4 years ago

Hi @eccentric-j if you ended up fixing the problem, can you please add a comment to this issue of what the problem and fix were? In case other people have similar problems and search the issue tracker. Thanks, Alex.

jaidetree commented 4 years ago

Unfortunately no such luck. I'm back to square one. Python is working, but unfortunately not SQL:

2020-08-19 22 18 16

Trying to think through where to even look, considering possibilities:

  1. I'm still not configuring sql-indent correctly even though it works in SQL buffers including using C-' in SQL src blocks. Seems like the most plausible yet there's only 1 line config required which I've included my current config in the gif
  2. Doom emacs is adding config that is causing indent to behave differently. This doesn't seem likely either given that Python's indent is working, but on the other hand, SQL was working for you once you enabled those core src block formatting settings.
  3. A SQL indent issue. Seems even less likely than the other two possibilities given that it was indenting correctly for you in SQL src blocks.
  4. An org-mode issue. Seems slightly possible, but it seems Python and JS indent correctly for me.

Where would you (or anyone here) start?

alex-hhh commented 4 years ago

You can use the following steps to check if there is an interference from another package or configuration setting:

(require 'sql-indent)
(add-hook 'sql-mode-hook 'sqlind-minor-mode)
(setq org-src-tab-acts-natively  t)
(setq org-src-preserve-indentation t)
(setq org-src-fontify-natively t)

and try to see if this is working, if it does, the problem is somewhere in your configuration or other loaded packages.

Also, if the (require 'sql-indent) fails, you will need to update the load path using:

(setq load-path (cons "PATH-TO-SQL-INDENT-PACKAGE-HERE" load-path)
jaidetree commented 4 years ago

Thanks for that tip! It did work correctly in a new org mode buffer so it's either something related to doom or my personal config. I know where to look now!

UPDATE Tried the above without calling (add-hook 'sql-mode-hook 'sqlind-minor-mode) and it STILL worked which means it's likely not my config. Either the package is not loading at the right time, or there's extra config that's conflicting.

On Wed, Aug 19, 2020 at 10:46 PM Alex Harsányi notifications@github.com wrote:

You can use the following steps to check if there is an interference from another package or configuration setting:

  • start emacs using emacs -Q --no-site-file -- this will start Emacs without loading any configuration files
  • Start an IELM buffer (type M-x ielm RET and type in the following:

(require 'sql-indent) (add-hook 'sql-mode-hook 'sqlind-minor-mode) (setq org-src-tab-acts-natively t) (setq org-src-preserve-indentation t) (setq org-src-fontify-natively t)

and try to see if this is working, if it does, the problem is somewhere in your configuration or other loaded packages.

Also, if the (require 'sql-indent) fails, you will need to update the load path using:

(setq load-path (cons "PATH-TO-SQL-INDENT-PACKAGE-HERE" load-path)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/alex-hhh/emacs-sql-indent/issues/96#issuecomment-676878696, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEQDWN4N46XZ2HCZZJYWWTSBSE7VANCNFSM4QFJOHAA .

jaidetree commented 4 years ago

Found the cause:

Doom has added advice to the editing of src blocks that prevents major mode hooks from firing which is why sqlind-minor-mode is not seemingly activating while editing src blocks.

My temporary solution:

(after! org
  (advice-remove #'org-src--edit-element #'+org-inhibit-mode-hooks-a))

But may degrade performance. Created a bug ticket, awaiting a better solution strategy. Thanks again for your help, it really helped me dig my way through.

alex-hhh commented 4 years ago

This was a good investigation! The reason why C, JS and Python work is because their indentation is provided by the default package (c-mode, js-mode or python-mode). for SQL indentation is not provided by sql-mode, but by this separate package, which needs to be activated when SQL mode is enabled for a buffer.

As for why this is a separate package, it is a long story (#5).

jaidetree commented 4 years ago

This was a good investigation! The reason why C, JS and Python work is because their indentation is provided by the default package (c-mode, js-mode or python-mode). for SQL indentation is not provided by sql-mode, but by this separate package, which needs to be activated when SQL mode is enabled for a buffer.

That's exactly what I was leaning towards as well.

As for why this is a separate package, it is a long story (#5).

Oof that's really too bad it all comes down to 1 person not liking it.

What options are available to address it given that it can't be merged upstream?

alex-hhh commented 4 years ago

That one person is the sql mode maintainer, so his opinion carries a lot of weight :-). In any case, sql-indent has been available as a package from ELPA for a while now, and will remain like this for the foreseeable future.

jaidetree commented 4 years ago

That's fair. I have no idea when it makes sense to have an informed maintainer making less-popular but potentially more sustainable decisions vs democratized project.

Anyway for anyone coming to this later the maintainer of Doom released an update with a fix https://github.com/hlissner/doom-emacs/commit/dddfd9a7b1b5563586ac08832e2edf21e8be5a4f.