441 lines
14 KiB
Markdown
441 lines
14 KiB
Markdown
# 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
|