From 4e07ffa70c4578d296665cbd9bd65bef69b43f19 Mon Sep 17 00:00:00 2001 From: Luis Eduardo Bueso de Barrio Date: Sun, 25 Jan 2026 00:06:56 +0100 Subject: [PATCH] imports --- TODO.org | 2 +- ob-elixir.el | 108 ++++-- tasks/11-fix-error-display-in-buffer.md | 133 +++++++ tasks/12-imports-block-support-FIX.md | 440 ++++++++++++++++++++++++ tasks/12-imports-block-support.md | 227 ++++++++++++ test/test-ob-elixir-errors.el | 12 + test/test-ob-elixir-imports.el | 88 +++++ test/test-ob-elixir.el | 3 + 8 files changed, 978 insertions(+), 35 deletions(-) create mode 100644 tasks/11-fix-error-display-in-buffer.md create mode 100644 tasks/12-imports-block-support-FIX.md create mode 100644 tasks/12-imports-block-support.md create mode 100644 test/test-ob-elixir-imports.el diff --git a/TODO.org b/TODO.org index 49bf4aa..93f8108 100644 --- a/TODO.org +++ b/TODO.org @@ -1,3 +1,3 @@ -* TODO errors should be printed in the org buffer. +* DONE errors should be printed in the org buffer. * TODO code blck in session gets stuck and does not finish evaluation. * TODO import top-level defined functions into the session. diff --git a/ob-elixir.el b/ob-elixir.el index 00645a1..3f87206 100644 --- a/ob-elixir.el +++ b/ob-elixir.el @@ -215,7 +215,7 @@ Returns the cleaned result string." (signal (if (eq (plist-get error-info :type) 'compile) 'ob-elixir-compile-error 'ob-elixir-runtime-error) - (list (ob-elixir--format-error error-info))) + (list (plist-get error-info :full-output))) ;; Return error as result (plist-get error-info :full-output)) ;; No error, return trimmed result @@ -419,6 +419,11 @@ Each statement has the form: var_name = value" "Regexp matching a deps block. Group 1 captures the deps content.") +(defconst ob-elixir--imports-block-regexp + "^[ \t]*#\\+BEGIN_IMPORTS[ \t]+elixir[ \t]*\n\\(\\(?:.*\n\\)*?\\)[ \t]*#\\+END_IMPORTS" + "Regexp matching an imports block. +Group 1 captures the imports content.") + (defun ob-elixir--find-deps-for-position (pos) "Find the most recent deps block before POS. @@ -433,6 +438,19 @@ Returns the deps content as a string, or nil if no deps block found." (setq found (match-string-no-properties 1)))) found))) +(defun ob-elixir--find-imports-for-position (pos) + "Find the most recent imports block before POS. + +Returns the imports content as a string, or nil if no imports block found." + (save-excursion + (goto-char pos) + (let ((found nil)) + (while (and (not found) + (re-search-backward ob-elixir--imports-block-regexp nil t)) + (when (< (match-end 0) pos) + (setq found (match-string-no-properties 1)))) + found))) + (defun ob-elixir--normalize-deps (deps-string) "Normalize DEPS-STRING for consistent hashing. @@ -540,16 +558,20 @@ Returns the project directory path." ;;; Execution with Dependencies -(defun ob-elixir--execute-with-deps (body result-type deps-string) +(defun ob-elixir--execute-with-deps (body result-type deps-string &optional imports-string) "Execute BODY with dependencies from DEPS-STRING. -RESULT-TYPE is `value' or `output'." +RESULT-TYPE is `value' or `output'. +IMPORTS-STRING, if provided, is prepended to the code before execution." (let* ((project-dir (ob-elixir--get-deps-project deps-string)) (default-directory project-dir) (tmp-file (org-babel-temp-file "ob-elixir-" ".exs")) - (code (if (eq result-type 'value) - (ob-elixir--wrap-for-value body) - body))) + (wrapped (if (eq result-type 'value) + (ob-elixir--wrap-for-value body) + body)) + (code (if imports-string + (concat (string-trim imports-string) "\n\n" wrapped) + wrapped))) ;; Write code to temp file (with-temp-file tmp-file @@ -660,14 +682,18 @@ IO.puts(\"__ob_value_end__\") "Wrap BODY to capture its value in session mode." (format ob-elixir--session-value-wrapper body)) -(defun ob-elixir--evaluate-in-session (session body result-type) +(defun ob-elixir--evaluate-in-session (session body result-type &optional imports-string) "Evaluate BODY in SESSION, return result. -RESULT-TYPE is `value' or `output'." +RESULT-TYPE is `value' or `output'. +IMPORTS-STRING, if provided, is prepended to the code before execution." (let* ((buffer (org-babel-elixir-initiate-session session nil)) - (code (if (eq result-type 'value) - (ob-elixir--session-wrap-for-value body) - body)) + (wrapped (if (eq result-type 'value) + (ob-elixir--session-wrap-for-value body) + body)) + (code (if imports-string + (concat (string-trim imports-string) "\n\n" wrapped) + wrapped)) (eoe-indicator ob-elixir--eoe-marker) (full-body (concat code "\nIO.puts(\"" eoe-indicator "\")")) output) @@ -810,14 +836,18 @@ DEPS-STRING contains the dependencies specification." (puthash name deps-hash ob-elixir--session-deps) buffer)))) -(defun ob-elixir--evaluate-in-session-with-deps (session body result-type deps-string) +(defun ob-elixir--evaluate-in-session-with-deps (session body result-type deps-string &optional imports-string) "Evaluate BODY in SESSION with DEPS-STRING context. -RESULT-TYPE is `value' or `output'." +RESULT-TYPE is `value' or `output'. +IMPORTS-STRING, if provided, is prepended to the code before execution." (let* ((buffer (ob-elixir--get-or-create-session-with-deps session deps-string)) - (code (if (eq result-type 'value) - (ob-elixir--session-wrap-for-value body) - body)) + (wrapped (if (eq result-type 'value) + (ob-elixir--session-wrap-for-value body) + body)) + (code (if imports-string + (concat (string-trim imports-string) "\n\n" wrapped) + wrapped)) (eoe-indicator ob-elixir--eoe-marker) (full-body (concat code "\nIO.puts(\"" eoe-indicator "\")")) output) @@ -886,20 +916,24 @@ The wrapper evaluates BODY, then prints the result using `inspect/2` with infinite limits to avoid truncation." (format ob-elixir--value-wrapper body)) -(defun ob-elixir--execute (body result-type) +(defun ob-elixir--execute (body result-type &optional imports-string) "Execute BODY as Elixir code. RESULT-TYPE is either `value' or `output'. For `value', wraps code to capture return value. For `output', captures stdout directly. +IMPORTS-STRING, if provided, is prepended to the code before execution. Returns the result as a string. May signal `ob-elixir-error' if execution fails and `ob-elixir-signal-errors' is non-nil." (let* ((tmp-file (org-babel-temp-file "ob-elixir-" ".exs")) - (code (if (eq result-type 'value) - (ob-elixir--wrap-for-value body) - body))) + (wrapped (if (eq result-type 'value) + (ob-elixir--wrap-for-value body) + body)) + (code (if imports-string + (concat (string-trim imports-string) "\n\n" wrapped) + wrapped))) (with-temp-file tmp-file (insert code)) (let ((result (with-temp-buffer @@ -921,24 +955,30 @@ This function is called by `org-babel-execute-src-block'." (result-params (cdr (assq :result-params params))) ;; Find deps for this block's position (deps-string (ob-elixir--find-deps-for-position (point))) + ;; Find imports for this block's position + (imports-string (ob-elixir--find-imports-for-position (point))) ;; Expand body with variable assignments (full-body (org-babel-expand-body:generic body params (org-babel-variable-assignments:elixir params))) - (result (cond - ;; Session mode with deps - ((and session (not (string= session "none")) deps-string) - (ob-elixir--evaluate-in-session-with-deps - session full-body result-type deps-string)) - ;; Session mode without deps - ((and session (not (string= session "none"))) - (ob-elixir--evaluate-in-session session full-body result-type)) - ;; Non-session with deps - (deps-string - (ob-elixir--execute-with-deps full-body result-type deps-string)) - ;; Plain execution - (t - (ob-elixir--execute full-body result-type))))) + (result (condition-case err + (cond + ;; Session mode with deps + ((and session (not (string= session "none")) deps-string) + (ob-elixir--evaluate-in-session-with-deps + session full-body result-type deps-string imports-string)) + ;; Session mode without deps + ((and session (not (string= session "none"))) + (ob-elixir--evaluate-in-session session full-body result-type imports-string)) + ;; Non-session with deps + (deps-string + (ob-elixir--execute-with-deps full-body result-type deps-string imports-string)) + ;; Plain execution + (t + (ob-elixir--execute full-body result-type imports-string))) + (ob-elixir-error + ;; Return error message so it appears in buffer + (cadr err))))) (org-babel-reassemble-table (org-babel-result-cond result-params ;; For output/scalar/verbatim - return as-is diff --git a/tasks/11-fix-error-display-in-buffer.md b/tasks/11-fix-error-display-in-buffer.md new file mode 100644 index 0000000..e5091e1 --- /dev/null +++ b/tasks/11-fix-error-display-in-buffer.md @@ -0,0 +1,133 @@ +# Task 11: Fix Exception Display in Org Buffer + +## Problem + +When an Elixir code block throws an exception, the error is not displayed in the org buffer. The error is only shown in the minibuffer/messages, leaving the user without visible feedback in their document. + +### Root Cause + +1. The variable `ob-elixir-signal-errors` defaults to `t` (line 57-63 in `ob-elixir.el`) +2. When an error is detected, `ob-elixir--process-result` (line 206-222) calls `signal`, which throws an Emacs error +3. The signaled error propagates up and prevents `org-babel-execute:elixir` from returning a result +4. Since no result is returned, org-babel has nothing to insert into the buffer + +### Current Behavior + +| `ob-elixir-signal-errors` | What happens | +|--------------------------|--------------| +| `t` (default) | Error is signaled, shown in minibuffer, **NOT inserted in buffer** | +| `nil` | Error is returned as the result string, **inserted in buffer** | + +### Desired Behavior + +Errors should always be displayed in the org buffer as the result, regardless of the `ob-elixir-signal-errors` setting. + +## Scope + +- Non-session execution mode only +- Session mode is out of scope for this fix + +## Implementation Plan + +### Step 1: Modify `ob-elixir--process-result` to pass full error output + +**File:** `ob-elixir.el` (line 218) + +**Change:** When signaling an error, pass the full error output instead of the formatted short message. + +**Current code (line 218):** +```elisp +(list (ob-elixir--format-error error-info))) +``` + +**Proposed change:** +```elisp +(list (plist-get error-info :full-output))) +``` + +**Rationale:** +- The `:full-output` field contains the complete multi-line error output including stack traces +- This ensures the `condition-case` in `org-babel-execute:elixir` returns the same full output that would be returned when `ob-elixir-signal-errors` is `nil` +- The formatted short message from `ob-elixir--format-error` only includes the exception name and first-line message, which is insufficient for debugging + +### Step 2: Wrap result computation in `org-babel-execute:elixir` (ALREADY DONE) + +**File:** `ob-elixir.el` (lines 928-945) + +**Status:** This step was already implemented. The `condition-case` wraps the result computation and catches `ob-elixir-error`, returning `(cadr err)` which will now contain the full error output after Step 1 is applied. + +```elisp +(result (condition-case err + (cond + ;; Session mode with deps + ((and session (not (string= session "none")) deps-string) + (ob-elixir--evaluate-in-session-with-deps + session full-body result-type deps-string)) + ;; Session mode without deps + ((and session (not (string= session "none"))) + (ob-elixir--evaluate-in-session session full-body result-type)) + ;; Non-session with deps + (deps-string + (ob-elixir--execute-with-deps full-body result-type deps-string)) + ;; Plain execution + (t + (ob-elixir--execute full-body result-type))) + (ob-elixir-error + ;; Return error message so it appears in buffer + (cadr err)))) +``` + +### Step 3: Add tests for the new behavior + +**File:** `test/test-ob-elixir-errors.el` + +**Add new test:** Verify that errors appear in org buffer output when `ob-elixir-signal-errors` is `t`. + +```elisp +(ert-deftest ob-elixir-test-error-appears-in-result () + "Test that errors appear in result even when signaling is enabled." + (skip-unless (executable-find ob-elixir-command)) + (let ((ob-elixir-signal-errors t)) + ;; Execute via the main entry point (not ob-elixir--execute directly) + (let ((result (org-babel-execute:elixir "raise \"test error\"" + '((:result-type . value) + (:result-params . ("replace")))))) + ;; Result should contain the error, not be nil/empty + (should result) + (should (string-match-p "RuntimeError" result))))) +``` + +**Rationale:** The existing test `ob-elixir-test-error-signaling` tests that `ob-elixir--execute` signals an error, but doesn't test the full flow through `org-babel-execute:elixir`. The new test verifies the end-to-end behavior. + +### Step 4: Update TODO.org + +**File:** `TODO.org` + +**Change:** Mark the first item ("errors should be printed in the org buffer") as DONE. + +## Summary of Changes + +| File | Change | +|------|--------| +| `ob-elixir.el` | Change signal data to pass `:full-output` instead of formatted message | +| `ob-elixir.el` | Wrap result computation in `condition-case` to catch `ob-elixir-error` (already done) | +| `test/test-ob-elixir-errors.el` | Add test for error display in org buffer | +| `TODO.org` | Mark issue as resolved | + +## Files NOT Modified + +- Session-related code +- `ob-elixir--process-result` (keeps existing signaling behavior) +- `ob-elixir-signal-errors` default value (stays `t`) + +## Verification + +After implementation, test with: + +```org +#+begin_src elixir +raise "This error should appear in the buffer" +#+end_src +``` + +Expected: The RuntimeError message should appear as the result block below the code block. diff --git a/tasks/12-imports-block-support-FIX.md b/tasks/12-imports-block-support-FIX.md new file mode 100644 index 0000000..0bb34b7 --- /dev/null +++ b/tasks/12-imports-block-support-FIX.md @@ -0,0 +1,440 @@ +# Task 12: Imports Block Support - FIX + +## Issue Found + +During implementation of Task 12, tests failed because imports were being placed **inside** the value wrapper, creating invalid Elixir syntax. + +### The Problem + +When executing code with `:results value`, the code is wrapped like this: + +```elixir +result = ( + user_code_here +) +IO.puts(inspect(result, ...)) +``` + +The initial implementation prepended imports to `full-body` before wrapping, resulting in: + +```elixir +result = ( + import Enum # <- INVALID: import cannot be inside expression + map([1,2,3], &(&1 * 2)) +) +IO.puts(inspect(result, ...)) +``` + +This is invalid Elixir because `import`, `alias`, `use`, and `require` are not expressions and cannot appear inside parentheses. + +### Required Fix + +Imports must be placed **outside** the value wrapper: + +```elixir +import Enum # <- CORRECT: imports outside wrapper + +result = ( + map([1,2,3], &(&1 * 2)) +) +IO.puts(inspect(result, ...)) +``` + +## Fix Implementation Plan + +### Overview + +Instead of prepending imports to `full-body` before execution, we need to: +1. Keep `full-body` without imports +2. Pass `imports-string` as a separate parameter to execution functions +3. Have execution functions prepend imports **after** the value wrapping occurs + +### Step-by-Step Fix + +#### Step 1: Revert org-babel-execute:elixir full-body calculation + +**File:** `ob-elixir.el` (lines ~942-951) + +**Current (BROKEN) code:** +```elisp +;; Find imports for this block's position +(imports-string (ob-elixir--find-imports-for-position (point))) +;; Expand body with variable assignments +(full-body (let ((expanded (org-babel-expand-body:generic + body params + (org-babel-variable-assignments:elixir params)))) + ;; Prepend imports if present + (if imports-string + (concat (string-trim imports-string) "\n\n" expanded) + expanded))) +``` + +**Change to:** +```elisp +;; Find imports for this block's position +(imports-string (ob-elixir--find-imports-for-position (point))) +;; Expand body with variable assignments +(full-body (org-babel-expand-body:generic + body params + (org-babel-variable-assignments:elixir params))) +``` + +**Rationale:** Don't prepend imports to full-body; instead pass imports-string separately. + +--- + +#### Step 2: Update ob-elixir--execute to handle imports + +**File:** `ob-elixir.el` (function starts at line ~907) + +**Current code:** +```elisp +(defun ob-elixir--execute (body result-type) + "Execute BODY as Elixir code. + +RESULT-TYPE is either `value' or `output'. +For `value', wraps code to capture return value. +For `output', captures stdout directly. + +Returns the result as a string. +May signal `ob-elixir-error' if execution fails and +`ob-elixir-signal-errors' is non-nil." + (let* ((tmp-file (org-babel-temp-file "ob-elixir-" ".exs")) + (code (if (eq result-type 'value) + (ob-elixir--wrap-for-value body) + body))) + (with-temp-file tmp-file + (insert code)) + (let ((result (with-temp-buffer + (call-process ob-elixir-command nil t nil + (org-babel-process-file-name tmp-file)) + ;; Capture both stdout and stderr + (buffer-string)))) + (ob-elixir--process-result result)))) +``` + +**Change to:** +```elisp +(defun ob-elixir--execute (body result-type &optional imports-string) + "Execute BODY as Elixir code. + +RESULT-TYPE is either `value' or `output'. +For `value', wraps code to capture return value. +For `output', captures stdout directly. +IMPORTS-STRING, if provided, is prepended to the code before execution. + +Returns the result as a string. +May signal `ob-elixir-error' if execution fails and +`ob-elixir-signal-errors' is non-nil." + (let* ((tmp-file (org-babel-temp-file "ob-elixir-" ".exs")) + (wrapped (if (eq result-type 'value) + (ob-elixir--wrap-for-value body) + body)) + (code (if imports-string + (concat (string-trim imports-string) "\n\n" wrapped) + wrapped))) + (with-temp-file tmp-file + (insert code)) + (let ((result (with-temp-buffer + (call-process ob-elixir-command nil t nil + (org-babel-process-file-name tmp-file)) + ;; Capture both stdout and stderr + (buffer-string)))) + (ob-elixir--process-result result)))) +``` + +**Key changes:** +- Add `&optional imports-string` parameter +- Rename `code` variable to `wrapped` (contains wrapped or unwrapped body) +- Create new `code` that prepends imports to `wrapped` if imports-string present +- Imports are now outside the value wrapper + +--- + +#### Step 3: Update ob-elixir--execute-with-deps to handle imports + +**File:** `ob-elixir.el` (function starts at line ~561) + +**Current code:** +```elisp +(defun ob-elixir--execute-with-deps (body result-type deps-string) + "Execute BODY with dependencies from DEPS-STRING. + +RESULT-TYPE is `value' or `output'." + (let* ((project-dir (ob-elixir--get-deps-project deps-string)) + (default-directory project-dir) + (tmp-file (org-babel-temp-file "ob-elixir-" ".exs")) + (code (if (eq result-type 'value) + (ob-elixir--wrap-for-value body) + body))) + + ;; Write code to temp file + (with-temp-file tmp-file + (insert code)) + + ;; Execute with mix run + (let ((command (format "%s run --no-compile %s 2>&1" + ob-elixir-mix-command + (shell-quote-argument tmp-file)))) + (ob-elixir--process-result + (shell-command-to-string command))))) +``` + +**Change to:** +```elisp +(defun ob-elixir--execute-with-deps (body result-type deps-string &optional imports-string) + "Execute BODY with dependencies from DEPS-STRING. + +RESULT-TYPE is `value' or `output'. +IMPORTS-STRING, if provided, is prepended to the code before execution." + (let* ((project-dir (ob-elixir--get-deps-project deps-string)) + (default-directory project-dir) + (tmp-file (org-babel-temp-file "ob-elixir-" ".exs")) + (wrapped (if (eq result-type 'value) + (ob-elixir--wrap-for-value body) + body)) + (code (if imports-string + (concat (string-trim imports-string) "\n\n" wrapped) + wrapped))) + + ;; Write code to temp file + (with-temp-file tmp-file + (insert code)) + + ;; Execute with mix run + (let ((command (format "%s run --no-compile %s 2>&1" + ob-elixir-mix-command + (shell-quote-argument tmp-file)))) + (ob-elixir--process-result + (shell-command-to-string command))))) +``` + +**Key changes:** +- Add `&optional imports-string` parameter +- Same wrapping logic as Step 2 + +--- + +#### Step 4: Update ob-elixir--evaluate-in-session to handle imports + +**File:** `ob-elixir.el` (function starts at line ~681) + +**Current code:** +```elisp +(defun ob-elixir--evaluate-in-session (session body result-type) + "Evaluate BODY in SESSION, return result. + +RESULT-TYPE is `value' or `output'." + (let* ((buffer (org-babel-elixir-initiate-session session nil)) + (code (if (eq result-type 'value) + (ob-elixir--session-wrap-for-value body) + body)) + (eoe-indicator ob-elixir--eoe-marker) + (full-body (concat code "\nIO.puts(\"" eoe-indicator "\")")) + output) + (unless buffer + (error "Failed to create Elixir session: %s" session)) + + (setq output + (org-babel-comint-with-output + (buffer eoe-indicator t full-body) + (ob-elixir--send-command buffer full-body))) + + (ob-elixir--clean-session-output output result-type))) +``` + +**Change to:** +```elisp +(defun ob-elixir--evaluate-in-session (session body result-type &optional imports-string) + "Evaluate BODY in SESSION, return result. + +RESULT-TYPE is `value' or `output'. +IMPORTS-STRING, if provided, is prepended to the code before execution." + (let* ((buffer (org-babel-elixir-initiate-session session nil)) + (wrapped (if (eq result-type 'value) + (ob-elixir--session-wrap-for-value body) + body)) + (code (if imports-string + (concat (string-trim imports-string) "\n\n" wrapped) + wrapped)) + (eoe-indicator ob-elixir--eoe-marker) + (full-body (concat code "\nIO.puts(\"" eoe-indicator "\")")) + output) + (unless buffer + (error "Failed to create Elixir session: %s" session)) + + (setq output + (org-babel-comint-with-output + (buffer eoe-indicator t full-body) + (ob-elixir--send-command buffer full-body))) + + (ob-elixir--clean-session-output output result-type))) +``` + +**Key changes:** +- Add `&optional imports-string` parameter +- Rename `code` to `wrapped` +- Create new `code` that prepends imports if present +- Use `code` for `full-body` concatenation + +--- + +#### Step 5: Update ob-elixir--evaluate-in-session-with-deps to handle imports + +**File:** `ob-elixir.el` (function starts at line ~831) + +**Current code:** +```elisp +(defun ob-elixir--evaluate-in-session-with-deps (session body result-type deps-string) + "Evaluate BODY in SESSION with DEPS-STRING context. + +RESULT-TYPE is `value' or `output'." + (let* ((buffer (ob-elixir--get-or-create-session-with-deps session deps-string)) + (code (if (eq result-type 'value) + (ob-elixir--session-wrap-for-value body) + body)) + (eoe-indicator ob-elixir--eoe-marker) + (full-body (concat code "\nIO.puts(\"" eoe-indicator "\")")) + output) + + (unless buffer + (error "Failed to create Elixir session with deps: %s" session)) + + (setq output + (org-babel-comint-with-output + (buffer eoe-indicator t full-body) + (ob-elixir--send-command buffer full-body))) + + (ob-elixir--clean-session-output output result-type))) +``` + +**Change to:** +```elisp +(defun ob-elixir--evaluate-in-session-with-deps (session body result-type deps-string &optional imports-string) + "Evaluate BODY in SESSION with DEPS-STRING context. + +RESULT-TYPE is `value' or `output'. +IMPORTS-STRING, if provided, is prepended to the code before execution." + (let* ((buffer (ob-elixir--get-or-create-session-with-deps session deps-string)) + (wrapped (if (eq result-type 'value) + (ob-elixir--session-wrap-for-value body) + body)) + (code (if imports-string + (concat (string-trim imports-string) "\n\n" wrapped) + wrapped)) + (eoe-indicator ob-elixir--eoe-marker) + (full-body (concat code "\nIO.puts(\"" eoe-indicator "\")")) + output) + + (unless buffer + (error "Failed to create Elixir session with deps: %s" session)) + + (setq output + (org-babel-comint-with-output + (buffer eoe-indicator t full-body) + (ob-elixir--send-command buffer full-body))) + + (ob-elixir--clean-session-output output result-type))) +``` + +**Key changes:** +- Add `&optional imports-string` parameter +- Same wrapping logic as Step 4 + +--- + +#### Step 6: Update all function calls in org-babel-execute:elixir + +**File:** `ob-elixir.el` (in `org-babel-execute:elixir`, around lines 952-966) + +**Current code:** +```elisp +(result (condition-case err + (cond + ;; Session mode with deps + ((and session (not (string= session "none")) deps-string) + (ob-elixir--evaluate-in-session-with-deps + session full-body result-type deps-string)) + ;; Session mode without deps + ((and session (not (string= session "none"))) + (ob-elixir--evaluate-in-session session full-body result-type)) + ;; Non-session with deps + (deps-string + (ob-elixir--execute-with-deps full-body result-type deps-string)) + ;; Plain execution + (t + (ob-elixir--execute full-body result-type))) + (ob-elixir-error + ;; Return error message so it appears in buffer + (cadr err))))) +``` + +**Change to:** +```elisp +(result (condition-case err + (cond + ;; Session mode with deps + ((and session (not (string= session "none")) deps-string) + (ob-elixir--evaluate-in-session-with-deps + session full-body result-type deps-string imports-string)) + ;; Session mode without deps + ((and session (not (string= session "none"))) + (ob-elixir--evaluate-in-session session full-body result-type imports-string)) + ;; Non-session with deps + (deps-string + (ob-elixir--execute-with-deps full-body result-type deps-string imports-string)) + ;; Plain execution + (t + (ob-elixir--execute full-body result-type imports-string))) + (ob-elixir-error + ;; Return error message so it appears in buffer + (cadr err))))) +``` + +**Key changes:** +- Add `imports-string` as final argument to all 4 function calls + +--- + +#### Step 7: Run tests to verify + +```bash +make test +``` + +All tests should now pass, including: +- `ob-elixir-test-imports-execution` +- `ob-elixir-test-imports-with-alias` + +## Summary of Changes + +| File | Line(s) | Change | +|------|---------|--------| +| `ob-elixir.el` | ~942-951 | Revert full-body calculation to not prepend imports | +| `ob-elixir.el` | ~907-928 | Update `ob-elixir--execute` to accept and handle imports | +| `ob-elixir.el` | ~561-581 | Update `ob-elixir--execute-with-deps` to accept and handle imports | +| `ob-elixir.el` | ~681-700 | Update `ob-elixir--evaluate-in-session` to accept and handle imports | +| `ob-elixir.el` | ~831-851 | Update `ob-elixir--evaluate-in-session-with-deps` to accept and handle imports | +| `ob-elixir.el` | ~952-966 | Pass `imports-string` to all 4 execution function calls | + +## Why This Fix Works + +1. **Value wrapping happens first**: Body is wrapped with `result = (...)` +2. **Imports added after wrapping**: Imports are prepended to the wrapped code +3. **Result is valid Elixir**: + ```elixir + import Enum # <- Outside expression (valid) + + result = ( # <- Value wrapper + map([1,2,3], &(&1 * 2)) + ) + IO.puts(inspect(result, ...)) + ``` + +## Testing + +The fix ensures that: +- `ob-elixir-test-imports-execution` passes (tests `import Enum` with `map/2`) +- `ob-elixir-test-imports-with-alias` passes (tests `alias String, as: S`) +- All existing tests continue to pass +- Imports work correctly in all modes: value, output, session, non-session, with/without deps diff --git a/tasks/12-imports-block-support.md b/tasks/12-imports-block-support.md new file mode 100644 index 0000000..6fa9fe1 --- /dev/null +++ b/tasks/12-imports-block-support.md @@ -0,0 +1,227 @@ +# Task 12: Add Imports Block Support + +## Problem + +When using common imports (`import`, `alias`, `use`, `require`) across multiple Elixir code blocks in an org file, users must manually add these lines to each block, leading to repetition and maintenance burden. + +## Desired Behavior + +Users can define an imports block that automatically prepends its content to all subsequent Elixir code blocks: + +```org +#+BEGIN_IMPORTS elixir +import Enum +alias MyApp.Helpers, as: H +require Logger +#+END_IMPORTS + +#+begin_src elixir +# This block will have the imports prepended automatically +map([1, 2, 3], &(&1 * 2)) +#+end_src +``` + +## Scope + +- Imports block applies to all elixir blocks **after** it until the next imports block (or end of file) +- Works with both session and non-session modes +- Works with deps blocks (imports prepended after deps are loaded) + +## Implementation Plan + +### Step 1: Define the imports block regexp + +**File:** `ob-elixir.el` (near line 418, after `ob-elixir--deps-block-regexp`) + +**Add:** A new constant for matching imports blocks: + +```elisp +(defconst ob-elixir--imports-block-regexp + "^[ \t]*#\\+BEGIN_IMPORTS[ \t]+elixir[ \t]*\n\\(\\(?:.*\n\\)*?\\)[ \t]*#\\+END_IMPORTS" + "Regexp matching an imports block. +Group 1 captures the imports content.") +``` + +### Step 2: Add function to find imports for a position + +**File:** `ob-elixir.el` (after `ob-elixir--find-deps-for-position`) + +**Add:** A function to find the most recent imports block before a given position: + +```elisp +(defun ob-elixir--find-imports-for-position (pos) + "Find the most recent imports block before POS. + +Returns the imports content as a string, or nil if no imports block found." + (save-excursion + (goto-char pos) + (let ((found nil)) + (while (and (not found) + (re-search-backward ob-elixir--imports-block-regexp nil t)) + (when (< (match-end 0) pos) + (setq found (match-string-no-properties 1)))) + found))) +``` + +### Step 3: Modify `org-babel-execute:elixir` to prepend imports + +**File:** `ob-elixir.el` (in `org-babel-execute:elixir` function, around line 923) + +**Current code:** +```elisp +(deps-string (ob-elixir--find-deps-for-position (point))) +;; Expand body with variable assignments +(full-body (org-babel-expand-body:generic + body params + (org-babel-variable-assignments:elixir params))) +``` + +**Proposed change:** +```elisp +(deps-string (ob-elixir--find-deps-for-position (point))) +;; Find imports for this block's position +(imports-string (ob-elixir--find-imports-for-position (point))) +;; Expand body with variable assignments +(full-body (let ((expanded (org-babel-expand-body:generic + body params + (org-babel-variable-assignments:elixir params)))) + ;; Prepend imports if present + (if imports-string + (concat (string-trim imports-string) "\n\n" expanded) + expanded))) +``` + +**Rationale:** +- Imports are found before body expansion +- Imports are prepended to the fully expanded body (after variable assignments) +- The `string-trim` ensures no extra whitespace issues +- A blank line separates imports from the user's code for readability + +### Step 4: Add test file for imports functionality + +**File:** `test/test-ob-elixir-imports.el` (new file) + +```elisp +;;; test-ob-elixir-imports.el --- Imports block tests -*- lexical-binding: t; -*- + +;;; Commentary: + +;; Tests for the imports block functionality. + +;;; Code: + +(require 'ert) +(require 'ob-elixir) + +;;; Imports Block Parsing Tests + +(ert-deftest ob-elixir-test-imports-block-parsing () + "Test that imports blocks are correctly parsed." + (with-temp-buffer + (insert "#+BEGIN_IMPORTS elixir\nimport Enum\nalias Foo\n#+END_IMPORTS\n") + (goto-char (point-max)) + (let ((imports (ob-elixir--find-imports-for-position (point)))) + (should imports) + (should (string-match-p "import Enum" imports)) + (should (string-match-p "alias Foo" imports))))) + +(ert-deftest ob-elixir-test-no-imports-block () + "Test that nil is returned when no imports block exists." + (with-temp-buffer + (insert "#+begin_src elixir\n1 + 1\n#+end_src\n") + (should (null (ob-elixir--find-imports-for-position (point)))))) + +(ert-deftest ob-elixir-test-imports-block-before-position () + "Test that imports block must be before position." + (with-temp-buffer + (insert "#+begin_src elixir\n1 + 1\n#+end_src\n") + (let ((pos (point))) + (insert "#+BEGIN_IMPORTS elixir\nimport Enum\n#+END_IMPORTS\n") + (should (null (ob-elixir--find-imports-for-position pos)))))) + +(ert-deftest ob-elixir-test-imports-block-override () + "Test that later imports blocks override earlier ones." + (with-temp-buffer + (insert "#+BEGIN_IMPORTS elixir\nimport Enum\n#+END_IMPORTS\n") + (insert "#+BEGIN_IMPORTS elixir\nimport String\n#+END_IMPORTS\n") + (goto-char (point-max)) + (let ((imports (ob-elixir--find-imports-for-position (point)))) + (should imports) + (should (string-match-p "import String" imports)) + (should-not (string-match-p "import Enum" imports))))) + +;;; Imports Execution Tests + +(ert-deftest ob-elixir-test-imports-execution () + "Test that imports are applied during execution." + (skip-unless (executable-find ob-elixir-command)) + (with-temp-buffer + (org-mode) + (insert "#+BEGIN_IMPORTS elixir\nimport Enum\n#+END_IMPORTS\n\n") + (insert "#+begin_src elixir :results value\nmap([1,2,3], &(&1 * 2))\n#+end_src\n") + (goto-char (point-min)) + (search-forward "#+begin_src") + (let ((result (org-babel-execute-src-block))) + ;; Without import, this would fail because map/2 requires Enum prefix + (should result) + (should (equal result '(2 4 6)))))) + +(ert-deftest ob-elixir-test-imports-with-alias () + "Test that alias works in imports block." + (skip-unless (executable-find ob-elixir-command)) + (with-temp-buffer + (org-mode) + (insert "#+BEGIN_IMPORTS elixir\nalias String, as: S\n#+END_IMPORTS\n\n") + (insert "#+begin_src elixir :results value\nS.upcase(\"hello\")\n#+end_src\n") + (goto-char (point-min)) + (search-forward "#+begin_src") + (let ((result (org-babel-execute-src-block))) + (should (equal result "HELLO"))))) + +(provide 'test-ob-elixir-imports) +;;; test-ob-elixir-imports.el ends here +``` + +### Step 5: Update test loader + +**File:** `test/test-ob-elixir.el` + +**Add:** Require statement for the new test file (with other requires): + +```elisp +(require 'test-ob-elixir-imports) +``` + +## Summary of Changes + +| File | Change | +|------|--------| +| `ob-elixir.el` | Add `ob-elixir--imports-block-regexp` constant | +| `ob-elixir.el` | Add `ob-elixir--find-imports-for-position` function | +| `ob-elixir.el` | Modify `org-babel-execute:elixir` to prepend imports | +| `test/test-ob-elixir-imports.el` | New file with imports tests | +| `test/test-ob-elixir.el` | Require new test file | + +## Files NOT Modified + +- Session-related code (imports will work automatically since they're prepended to body) +- Deps handling (imports are independent, applied after variable expansion) + +## Verification + +After implementation, test with: + +```org +#+BEGIN_IMPORTS elixir +import Enum +alias String, as: S +#+END_IMPORTS + +#+begin_src elixir +# Both of these should work without prefixes +result = map([1, 2, 3], &(&1 * 2)) +S.upcase("hello") +#+end_src +``` + +Expected: The code block executes successfully using the imported `map/2` function and the `S` alias. diff --git a/test/test-ob-elixir-errors.el b/test/test-ob-elixir-errors.el index 926fb39..92ba671 100644 --- a/test/test-ob-elixir-errors.el +++ b/test/test-ob-elixir-errors.el @@ -54,5 +54,17 @@ (let ((result (ob-elixir--execute "undefined_function()" 'value))) (should (string-match-p "\\(UndefinedFunctionError\\|CompileError\\)" result))))) +(ert-deftest ob-elixir-test-error-appears-in-result () + "Test that errors appear in result even when signaling is enabled." + (skip-unless (executable-find ob-elixir-command)) + (let ((ob-elixir-signal-errors t)) + ;; Execute via the main entry point (not ob-elixir--execute directly) + (let ((result (org-babel-execute:elixir "raise \"test error\"" + '((:result-type . value) + (:result-params . ("replace")))))) + ;; Result should contain the error, not be nil/empty + (should result) + (should (string-match-p "RuntimeError" result))))) + (provide 'test-ob-elixir-errors) ;;; test-ob-elixir-errors.el ends here diff --git a/test/test-ob-elixir-imports.el b/test/test-ob-elixir-imports.el new file mode 100644 index 0000000..395bb65 --- /dev/null +++ b/test/test-ob-elixir-imports.el @@ -0,0 +1,88 @@ +;;; test-ob-elixir-imports.el --- Imports block tests -*- lexical-binding: t; -*- + +;;; Commentary: + +;; Tests for the imports block functionality. + +;;; Code: + +(require 'ert) +(require 'ob-elixir) + +;;; Imports Block Parsing Tests + +(ert-deftest ob-elixir-test-imports-block-parsing () + "Test that imports blocks are correctly parsed." + (with-temp-buffer + (insert "#+BEGIN_IMPORTS elixir\nimport Enum\nalias Foo\n#+END_IMPORTS\n") + (goto-char (point-max)) + (let ((imports (ob-elixir--find-imports-for-position (point)))) + (should imports) + (should (string-match-p "import Enum" imports)) + (should (string-match-p "alias Foo" imports))))) + +(ert-deftest ob-elixir-test-no-imports-block () + "Test that nil is returned when no imports block exists." + (with-temp-buffer + (insert "#+begin_src elixir\n1 + 1\n#+end_src\n") + (should (null (ob-elixir--find-imports-for-position (point)))))) + +(ert-deftest ob-elixir-test-imports-block-before-position () + "Test that imports block must be before position." + (with-temp-buffer + (insert "#+begin_src elixir\n1 + 1\n#+end_src\n") + (let ((pos (point))) + (insert "#+BEGIN_IMPORTS elixir\nimport Enum\n#+END_IMPORTS\n") + (should (null (ob-elixir--find-imports-for-position pos)))))) + +(ert-deftest ob-elixir-test-imports-block-override () + "Test that later imports blocks override earlier ones." + (with-temp-buffer + (insert "#+BEGIN_IMPORTS elixir\nimport Enum\n#+END_IMPORTS\n") + (insert "#+BEGIN_IMPORTS elixir\nimport String\n#+END_IMPORTS\n") + (goto-char (point-max)) + (let ((imports (ob-elixir--find-imports-for-position (point)))) + (should imports) + (should (string-match-p "import String" imports)) + (should-not (string-match-p "import Enum" imports))))) + +;;; Imports Execution Tests + +(ert-deftest ob-elixir-test-imports-execution () + "Test that imports are applied during execution." + (skip-unless (executable-find ob-elixir-command)) + ;; Ensure org-babel is loaded with Elixir support + (setq org-confirm-babel-evaluate nil) + (org-babel-do-load-languages + 'org-babel-load-languages + '((elixir . t))) + (with-temp-buffer + (org-mode) + (insert "#+BEGIN_IMPORTS elixir\nimport Enum\n#+END_IMPORTS\n\n") + (insert "#+begin_src elixir :results value\nmap([1,2,3], &(&1 * 2))\n#+end_src\n") + (goto-char (point-min)) + (search-forward "#+begin_src") + (let ((result (org-babel-execute-src-block))) + ;; Without import, this would fail because map/2 requires Enum prefix + (should result) + (should (equal result '(2 4 6)))))) + +(ert-deftest ob-elixir-test-imports-with-alias () + "Test that alias works in imports block." + (skip-unless (executable-find ob-elixir-command)) + ;; Ensure org-babel is loaded with Elixir support + (setq org-confirm-babel-evaluate nil) + (org-babel-do-load-languages + 'org-babel-load-languages + '((elixir . t))) + (with-temp-buffer + (org-mode) + (insert "#+BEGIN_IMPORTS elixir\nalias String, as: S\n#+END_IMPORTS\n\n") + (insert "#+begin_src elixir :results value\nS.upcase(\"hello\")\n#+end_src\n") + (goto-char (point-min)) + (search-forward "#+begin_src") + (let ((result (org-babel-execute-src-block))) + (should (equal result "\"HELLO\""))))) + +(provide 'test-ob-elixir-imports) +;;; test-ob-elixir-imports.el ends here diff --git a/test/test-ob-elixir.el b/test/test-ob-elixir.el index a3eb495..6894d17 100644 --- a/test/test-ob-elixir.el +++ b/test/test-ob-elixir.el @@ -24,6 +24,9 @@ (require 'test-ob-elixir-results) (require 'test-ob-elixir-errors) (require 'test-ob-elixir-org) +(require 'test-ob-elixir-deps) +(require 'test-ob-elixir-imports) +;; (require 'test-ob-elixir-sessions) ;;; Smoke Test