-
Notifications
You must be signed in to change notification settings - Fork 32
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
Let user control fallback to original babel sql exection function #74
Let user control fallback to original babel sql exection function #74
Conversation
@stardiviner, reverted since it can't be used in the case when Furthermore, why do you use (if (or (not ejc-org-mode-babel-wrapper)
(cdr (assq :engine params))) I guess
is enough. If one want to use ejc-sql, he can enabe it, if won't - disable. Please, consider both cases, when |
About this, I think because user might mix two types sql blocks in one Org buffer, for example, there are some sql blocks want need to specify
WDYT? I hope
This case, I think If you think this is ok, any suggestion welcome. I will make a new PR. |
Here is the updated logic: @@ -341,8 +341,9 @@ Prepare buffer to operate as `ejc-sql-mode' buffer."
(defun ejc-eval-org-snippet (&optional orig-fun body params)
"Used to eval SQL code in `org-mode' code snippets."
- (if (or (not ejc-org-mode-babel-wrapper)
- (cdr (assq :engine params)))
+ (if (and (not ejc-org-mode-babel-wrapper)
+ (cdr (assq :engine params))
+ (yes-or-no-p "ejc-sql is enabled, use it to execute sql block? "))
(funcall orig-fun body params)
(let* ((beg (save-excursion
(goto-char (nth 5 (org-babel-get-src-block-info)))
WDYT? |
Yes, it's perfectly ok. About choosing execution engine logic:
After all, I think this matches logic described above: (if (or (not ejc-org-mode-babel-wrapper)
(and (cdr (assq :engine params))
(not
(yes-or-no-p
(concat "ejc-sql is enabled, ignore block connection"
" params and use ejc-sql to execute it? ")))))
(funcall orig-fun body params)
(let* ((beg (save-excursion
... Also, please make a note about |
Ok, I updated, also mentioned this choosing engine logic in README, also add a link to this discussion. I checked out |
Unfortunately, I already merged and reversed it, so looks like a new pull request is required. |
User might want to use default sql exection function when mixing SQL code in one Org file. Or in some special cases.