Fix tracing in the run_docker_compare.bash script.

It is still testing the same thing, but I changed the contents a bit to match what is being used in my blog post.

.dockerignore

@@ -1,3 +1,4 @@
 **/.git
 target
 Cargo.lock
+notes/

.lighthouse/pipeline-rust-build.yaml

@@ -14,10 +14,6 @@ spec:
       - name: path-to-dockerfile
         description: The path to the Dockerfile
         type: string
-      - name: command
-        type: array
-        description: Command to run.
-        default: []
     tasks:
       - name: report-pending
         taskRef:
@@ -92,8 +88,6 @@ spec:
         runAfter:
           - build-image
         params:
-          - name: command
-            value: ["$(params.command[*])"]
           - name: args
             value: ["--no-default-features"]
           - name: docker-image
@@ -109,8 +103,6 @@ spec:
         runAfter:
           - run-image-none
         params:
-          - name: command
-            value: ["$(params.command[*])"]
           - name: args
             value: ["--no-default-features", "--features", "tracing"]
           - name: docker-image
@@ -126,8 +118,6 @@ spec:
         runAfter:
           - run-image-tracing
         params:
-          - name: command
-            value: ["$(params.command[*])"]
           - name: args
             value: ["--no-default-features", "--features", "compare"]
           - name: docker-image
@@ -143,8 +133,6 @@ spec:
         runAfter:
           - run-image-compare
         params:
-          - name: command
-            value: ["$(params.command[*])"]
           - name: args
             value: []
           - name: docker-image
@@ -160,8 +148,6 @@ spec:
         runAfter:
           - run-image-default
         params:
-          - name: command
-            value: ["$(params.command[*])"]
           - name: args
             value: ["--no-default-features", "--features", "tracing,compare"]
           - name: docker-image
@@ -256,5 +242,3 @@ spec:
       value: docker/organic_build/
     - name: path-to-dockerfile
       value: docker/organic_build/Dockerfile
-    - name: command
-      value: [cargo, build]

.lighthouse/pipeline-rust-test.yaml

@@ -4,6 +4,10 @@ metadata:
   name: rust-test
 spec:
   pipelineSpec:
+    timeouts:
+      pipeline: "2h0m0s"
+      tasks: "1h0m40s"
+      finally: "0h30m0s"
     params:
       - name: image-name
         description: The name for the built image
@@ -14,14 +18,6 @@ spec:
       - name: path-to-dockerfile
         description: The path to the Dockerfile
         type: string
-      - name: command
-        type: array
-        description: Command to run.
-        default: []
-      - name: args
-        type: array
-        description: Arguments passed to command.
-        default: []
     tasks:
       - name: do-stuff
         taskSpec:
@@ -113,10 +109,8 @@ spec:
         runAfter:
           - build-image
         params:
-          - name: command
-            value: ["$(params.command[*])"]
           - name: args
-            value: ["$(params.args[*])"]
+            value: [--no-fail-fast, --lib, --test, test_loader]
           - name: docker-image
             value: "$(params.image-name):$(tasks.fetch-repository.results.commit)"
     finally:
@@ -201,7 +195,6 @@ spec:
       secret:
         secretName: harbor-plain
   serviceAccountName: build-bot
-  timeout: 240h0m0s
   params:
     - name: image-name
       value: "harbor.fizz.buzz/private/organic-test"
@@ -209,7 +202,3 @@ spec:
       value: docker/organic_test/
     - name: path-to-dockerfile
       value: docker/organic_test/Dockerfile
-    - name: command
-      value: [cargo, test]
-    - name: args
-      value: [--lib, --test, test_loader]

.lighthouse/pipeline-rustfmt.yaml

@@ -14,14 +14,6 @@ spec:
       - name: path-to-dockerfile
         description: The path to the Dockerfile
         type: string
-      - name: rustfmt-command
-        type: array
-        description: Command to run rustfmt.
-        default: []
-      - name: rustfmt-args
-        type: array
-        description: Arguments passed to rustfmt.
-        default: []
       - name: GIT_USER_NAME
         description: The username for git
         type: string
@@ -119,10 +111,6 @@ spec:
         runAfter:
           - build-image
         params:
-          - name: command
-            value: ["$(params.rustfmt-command[*])"]
-          - name: args
-            value: ["$(params.rustfmt-args[*])"]
           - name: docker-image
             value: "$(params.image-name):$(tasks.fetch-repository.results.commit)"
       - name: cargo-fix
@@ -240,7 +228,3 @@ spec:
       value: docker/cargo_fmt/
     - name: path-to-dockerfile
       value: docker/cargo_fmt/Dockerfile
-    - name: command
-      value: [cargo, fmt]
-    - name: args
-      value: []

Cargo.toml

@@ -10,6 +10,12 @@ readme = "README.md"
 keywords = ["emacs", "org-mode"]
 categories = ["parsing"]
 resolver = "2"
+include = [
+  "LICENSE",
+  "**/*.rs",
+  "Cargo.toml",
+  "tests/*"
+]
 
 [lib]
 name = "organic"
@@ -34,10 +40,18 @@ tracing-subscriber = { version = "0.3.17", optional = true, features = ["env-fil
 walkdir = "2.3.3"
 
 [features]
-default = ["compare", "tracing"]
+default = ["compare"]
 compare = []
 tracing = ["dep:opentelemetry", "dep:opentelemetry-otlp", "dep:opentelemetry-semantic-conventions", "dep:tokio", "dep:tracing", "dep:tracing-opentelemetry", "dep:tracing-subscriber"]
 
-[profile.release]
+# Optimized build for any sort of release.
+[profile.release-lto]
+inherits = "release"
 lto = true
 strip = "symbols"
+
+# Profile for performance testing with the "perf" tool. Notably keeps debug enabled and does not strip symbols to make reading the perf output easier.
+[profile.perf]
+inherits = "release"
+lto = true
+debug = true

Makefile

@@ -35,7 +35,17 @@ clean:
 
 .PHONY: test
 test:
-> cargo test --lib --test test_loader -- --test-threads $(TESTJOBS)
+> cargo test --no-fail-fast --lib --test test_loader -- --test-threads $(TESTJOBS)
+
+.PHONY: dockertest
+dockertest:
+> $(MAKE) -C docker/organic_test
+> docker run --init --rm -i -t -v "$$(readlink -f ./):/source:ro" --mount source=cargo-cache,target=/usr/local/cargo/registry --mount source=rust-cache,target=/target --env CARGO_TARGET_DIR=/target -w /source organic-test --no-fail-fast --lib --test test_loader -- --test-threads $(TESTJOBS)
+
+.PHONY: dockerclean
+dockerclean:
+# Delete volumes created for running the tests in docker. This does not touch anything related to the jaeger docker container.
+> docker volume rm cargo-cache rust-cache
 
 .PHONY: integrationtest
 integrationtest:
@@ -49,8 +59,8 @@ unittest:
 jaeger:
 # 4317 for OTLP gRPC, 4318 for OTLP HTTP. We currently use gRPC but I forward both ports regardless.
 #
-# These flags didn't help even though they seem like they would: --collector.otlp.grpc.max-message-size=10000000 --collector.queue-size=20000 --collector.num-workers=100
+# These flags didn't help even though they seem like they would: --collector.queue-size=20000 --collector.num-workers=100
-> docker run -d --rm --name organicdocker -p 4317:4317 -p 4318:4318 -p 16686:16686 -e COLLECTOR_OTLP_ENABLED=true jaegertracing/all-in-one:1.47 --collector.grpc-server.max-message-size=10000000
+> docker run -d --rm --name organicdocker -p 4317:4317 -p 4318:4318 -p 16686:16686 -e COLLECTOR_OTLP_ENABLED=true jaegertracing/all-in-one:1.47 --collector.grpc-server.max-message-size=20000000 --collector.otlp.grpc.max-message-size=20000000
 
 .PHONY: jaegerweb
 jaegerweb:

build.rs

@@ -41,6 +41,7 @@ fn write_test(test_file: &mut File, test: &walkdir::DirEntry) {
         .strip_suffix(".org")
         .expect("Should have .org extension")
         .replace("/", "_");
+    let test_name = format!("autogen_{}", test_name);
 
     if let Some(_reason) = is_expect_fail(test_name.as_str()) {
         write!(test_file, "#[ignore]\n").unwrap();
@@ -71,10 +72,9 @@ use organic::parser::sexp::sexp_with_padding;
 
 fn is_expect_fail(name: &str) -> Option<&str> {
     match name {
-        "drawer_drawer_with_headline_inside" => Some("Apparently lines with :end: become their own paragraph. This odd behavior needs to be investigated more."),
+        "autogen_greater_element_drawer_drawer_with_headline_inside" => Some("Apparently lines with :end: become their own paragraph. This odd behavior needs to be investigated more."),
-        "element_container_priority_footnote_definition_dynamic_block" => Some("Apparently broken begin lines become their own paragraph."),
+        "autogen_element_container_priority_footnote_definition_dynamic_block" => Some("Apparently broken begin lines become their own paragraph."),
-        "paragraphs_paragraph_with_backslash_line_breaks" => Some("The text we're getting out of the parse tree is already processed to remove line breaks, so our comparison needs to take that into account."),
+        "autogen_lesser_element_paragraphs_paragraph_with_backslash_line_breaks" => Some("The text we're getting out of the parse tree is already processed to remove line breaks, so our comparison needs to take that into account."),
-        "export_snippet_paragraph_break_precedent" => Some("Emacs 28 has broken behavior so the tests in the CI fail."),
         _ => None,
     }
 }

docker/cargo_fmt/Makefile

@@ -6,7 +6,7 @@ all: build push
 
 .PHONY: build
 build:
-	docker build -t $(IMAGE_NAME) -f Dockerfile ../
+	docker build -t $(IMAGE_NAME) -f Dockerfile ../../
 
 .PHONY: push
 push:
@@ -26,10 +26,11 @@ else
 	@echo "REMOTE_REPO not defined, not removing from remote repo."
 endif
 
+# NOTE: This target will write to folders underneath the git-root
 .PHONY: run
-run:
+run: build
-	docker run --rm -i -t $(IMAGE_NAME)
+	docker run --rm --init -v "$$(readlink -f ../../):/source" --workdir=/source $(IMAGE_NAME)
 
 .PHONY: shell
-shell:
+shell: build
-	docker run --rm -i -t --entrypoint /bin/bash $(IMAGE_NAME)
+	docker run --rm -i -t --entrypoint /bin/sh -v "$$(readlink -f ../../):/source" --workdir=/source $(IMAGE_NAME)

docker/organic_build/Dockerfile

@@ -2,3 +2,5 @@ FROM rustlang/rust:nightly-alpine3.17
 
 RUN apk add --no-cache musl-dev
 RUN cargo install --locked --no-default-features --features ci-autoclean cargo-cache
+
+ENTRYPOINT ["cargo", "build"]

docker/organic_build/Makefile

@@ -6,7 +6,7 @@ all: build push
 
 .PHONY: build
 build:
-	docker build -t $(IMAGE_NAME) -f Dockerfile ../
+	docker build -t $(IMAGE_NAME) -f Dockerfile ../../
 
 .PHONY: push
 push:
@@ -25,11 +25,13 @@ ifdef REMOTE_REPO
 else
 	@echo "REMOTE_REPO not defined, not removing from remote repo."
 endif
+	docker volume rm cargo-cache
 
+# NOTE: This target will write to folders underneath the git-root
 .PHONY: run
-run:
+run: build
-	docker run --rm -i -t $(IMAGE_NAME)
+	docker run --rm --init -v "$$(readlink -f ../../):/source" --workdir=/source --mount source=cargo-cache,target=/usr/local/cargo/registry $(IMAGE_NAME)
 
 .PHONY: shell
-shell:
+shell: build
-	docker run --rm -i -t --entrypoint /bin/bash $(IMAGE_NAME)
+	docker run --rm -i -t --entrypoint /bin/sh -v "$$(readlink -f ../../):/source" --workdir=/source --mount source=cargo-cache,target=/usr/local/cargo/registry $(IMAGE_NAME)

docker/organic_test/Dockerfile

@@ -1,4 +1,34 @@
-FROM rustlang/rust:nightly-alpine3.17
+FROM alpine:3.17 AS build
+RUN apk add --no-cache build-base musl-dev git autoconf make texinfo gnutls-dev ncurses-dev gawk
 
-RUN apk add --no-cache musl-dev emacs
+
+FROM build AS build-emacs
+ARG EMACS_VERSION=emacs-29.1
+RUN git clone --depth 1 --branch $EMACS_VERSION https://git.savannah.gnu.org/git/emacs.git /root/emacs
+WORKDIR /root/emacs
+RUN mkdir /root/dist
+RUN ./autogen.sh
+RUN ./configure --prefix /usr --without-x --without-sound
+RUN make
+RUN make DESTDIR="/root/dist" install
+
+
+FROM build AS build-org-mode
+ARG ORG_VERSION=7bdec435ff5d86220d13c431e799c5ed44a57da1
+COPY --from=build-emacs /root/dist/ /
+RUN mkdir /root/dist
+# Savannah does not allow fetching specific revisions, so we're going to have to put unnecessary load on their server by cloning main and then checking out the revision we want.
+RUN git clone https://git.savannah.gnu.org/git/emacs/org-mode.git /root/org-mode && git -C /root/org-mode checkout $ORG_VERSION
+# RUN mkdir /root/org-mode && git -C /root/org-mode init --initial-branch=main && git -C /root/org-mode remote add origin https://git.savannah.gnu.org/git/emacs/org-mode.git && git -C /root/org-mode fetch origin $ORG_VERSION && git -C /root/org-mode checkout FETCH_HEAD
+WORKDIR /root/org-mode
+RUN make compile
+RUN make DESTDIR="/root/dist" install
+
+
+FROM rustlang/rust:nightly-alpine3.17
+RUN apk add --no-cache musl-dev ncurses gnutls
 RUN cargo install --locked --no-default-features --features ci-autoclean cargo-cache
+COPY --from=build-emacs /root/dist/ /
+COPY --from=build-org-mode /root/dist/ /
+
+ENTRYPOINT ["cargo", "test"]

docker/organic_test/Makefile

@@ -6,7 +6,7 @@ all: build push
 
 .PHONY: build
 build:
-	docker build -t $(IMAGE_NAME) -f Dockerfile ../
+	docker build -t $(IMAGE_NAME) -f Dockerfile ../../
 
 .PHONY: push
 push:
@@ -25,11 +25,12 @@ ifdef REMOTE_REPO
 else
 	@echo "REMOTE_REPO not defined, not removing from remote repo."
 endif
+	docker volume rm rust-cache cargo-cache
 
 .PHONY: run
-run:
+run: build
-	docker run --rm -i -t $(IMAGE_NAME)
+	docker run --rm --init -v "$$(readlink -f ../../):/source:ro" --workdir=/source --mount source=cargo-cache,target=/usr/local/cargo/registry --mount source=rust-cache,target=/target --env CARGO_TARGET_DIR=/target $(IMAGE_NAME) --no-fail-fast --lib --test test_loader
 
 .PHONY: shell
-shell:
+shell: build
-	docker run --rm -i -t --entrypoint /bin/bash $(IMAGE_NAME)
+	docker run --rm -i -t --entrypoint /bin/sh -v "$$(readlink -f ../../):/source:ro" --workdir=/source --mount source=cargo-cache,target=/usr/local/cargo/registry --mount source=rust-cache,target=/target --env CARGO_TARGET_DIR=/target $(IMAGE_NAME)

notes/optimization_ideas.org

@@ -0,0 +1,27 @@
+* Analysis
+** Parse start per character
+It might help analysis to record how often we start a specific type of parse for each character. For example, at the start of a plain list, if we had a count of how often each character was the start of a parse of a list we could use that to see how often that list is getting re-parsed.
+* Optimizations
+** Edit whitespace for list items
+Whether or not a list item owns the trailing whitespace depends on if it is the last list item in that list. Since we do not know ahead of time if an item is the last item in the list, we have to either re-parse the list item or modify it after parsing.
+
+*** For
+We already are modifying the source of some elements after-the-fact with src_rust{set_source()} so this would be more of the same.
+*** Against
+I'd like to phase out such modifications because they seem hacky and fragile.
+** Make detect element function
+Some exit matchers are based on when the next element is found. Some elements do not need to be fully parsed to identify that they are a valid element. For example, src_org{1. foo} can already be identified as the start of a plain list (in the right context) without needing to parse the entire element.
+*** For
+Avoiding parsing the entire element for an exit matcher would reduce redundant parses.
+*** Against
+This adds code complexity and introduces the potential for bugs.
+
+How many elements can be reasonably early-detected? For example, src_org{#+begin_src foo} is not enough to detect the start of a source block because without the src_org{#+end_src} it is just plain text.
+** Grab multiple characters in plaintext parser before checking exit matcher
+Currently we check the exit matcher after each character inside the plain text parser (and many others). Are there character sequences we can assume no exit matcher will trigger between? For example, a contiguous string of latin-alphabet letters?
+*** For
+This could significantly reduce our calls to exit matchers.
+*** Against
+I think targets would break this.
+
+The exit matchers are already implicitly building this behavior since they should all exit very early when the starting character is wrong. Putting this logic in a centralized place, far away from where those characters are actually going to be used, is unfortunate for readability.

org_mode_samples/element_container_priority/plain_list_greater_block.org

@@ -1,6 +1,7 @@
-1. foo
+1. plain-list
    #+begin_center
 
 
    #+end_center
-2. bar
+
+   Is this still in the plain list?

org_mode_samples/exit_matcher_investigation/README.txt

@@ -1 +0,0 @@
-This folder is an investigation into whether or not my exit matchers should operate from the top down or bottom up.

org_mode_samples/exit_matcher_investigation/bold_with_asterisk_inside.org

@@ -1 +0,0 @@
-foo *bar baz * lorem* ipsum

org_mode_samples/exit_matcher_investigation/list_paragraph_nesting.README.txt

@@ -1,3 +0,0 @@
-Looks like 2 blank lines always exits the top-level plain list.
-
-Plain lists do not seem to go inside paragraphs but rather exist beside them.

org_mode_samples/exit_matcher_investigation/list_paragraph_nesting.org

@@ -1,12 +0,0 @@
-1. foo
-
-   bar
-
-   1. baz
-
-      lorem
-
-   ipsum
-
-
-dolar

org_mode_samples/exit_matcher_investigation/table_list.README.txt

@@ -1 +0,0 @@
-Looks like table cells cannot contain lists but can contain bolds

org_mode_samples/exit_matcher_investigation/table_list.org

@@ -1,5 +0,0 @@
-ip *su* m
-
-| foo      | bar |
-|----------+-----|
-| 1. lo *re* m |     |

org_mode_samples/greater_element/dynamic_block/simple.org

@@ -0,0 +1,25 @@
+#+BEGIN: clocktable :scope file :maxlevel 2
+#+CAPTION: Clock summary at [2023-08-25 Fri 05:34]
+| Headline     | Time   |
+|--------------+--------|
+| *Total time* | *0:00* |
+#+END:
+
+#+BEGIN: columnview :hlines 1 :id global
+| ITEM  | TODO | PRIORITY | TAGS                         |
+|-------+------+----------+------------------------------|
+| Foo   |      | B        |                              |
+|-------+------+----------+------------------------------|
+| Bar   | TODO | B        |                              |
+|-------+------+----------+------------------------------|
+| Baz   |      | B        | :thisisatag:                 |
+| Lorem |      | B        | :thisshouldinheritfromabove: |
+| Ipsum |      | B        | :multiple:tags:              |
+#+END:
+* Foo
+* TODO Bar
+* Baz :thisisatag:
+** Lorem :thisshouldinheritfromabove:
+*** Ipsum                                                   :multiple:tags:
+* Dolar ::
+* cat :dog: bat

org_mode_samples/greater_element/plain_list/content_on_next_line.org

@@ -0,0 +1,7 @@
+1. foo
+2.
+     bar
+   1.
+       #+begin_center
+Still in the list
+       #+end_center

org_mode_samples/greater_element/plain_list/trailing_whitespace_ownership_test_case_1.org

@@ -0,0 +1,9 @@
+1. foo
+
+   1. bar
+
+   2. baz
+
+2. lorem
+
+ipsum

org_mode_samples/greater_element/plain_list/trailing_whitespace_ownership_test_case_2.org

@@ -0,0 +1,11 @@
+1. foo
+
+   1. bar
+
+   2. baz
+
+   cat
+
+2. lorem
+
+ipsum

org_mode_samples/greater_element/plain_list/trailing_whitespace_ownership_test_case_3.org

@@ -0,0 +1,10 @@
+1. cat
+   1. foo
+
+      1. bar
+
+      2. baz
+
+   2. lorem
+
+ipsum

org_mode_samples/greater_element/plain_list/trailing_whitespace_ownership_test_case_4.org

@@ -0,0 +1,12 @@
+1. cat
+   1. foo
+
+      1. bar
+
+      2. baz
+
+   2. lorem
+
+2. dog
+
+ipsum

org_mode_samples/trailing_whitespace/blank_lines_after_paragraphs.org

@@ -0,0 +1,18 @@
+foo bar.
+
+
+
+
+* Lorem
+baz
+
+
+
+
+* Ipsum
+alpha
+
+
+
+
+beta

scripts/callgrind.bash

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+#
+set -euo pipefail
+IFS=$'\n\t'
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+cd "$DIR/../"
+
+RUSTFLAGS="-C opt-level=0" cargo build --no-default-features
+valgrind --tool=callgrind --callgrind-out-file=callgrind.out target/debug/compare
+
+echo "You probably want to run:"
+echo "callgrind_annotate --auto=yes callgrind.out"

scripts/perf.bash

@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+#
+set -euo pipefail
+IFS=$'\n\t'
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+: ${PROFILE:="perf"}
+
+cd "$DIR/../"
+
+function main {
+    local additional_flags=()
+    if [ "$PROFILE" = "dev" ] || [ "$PROFILE" = "debug" ]; then
+        PROFILE="debug"
+    else
+        additional_flags+=(--profile "$PROFILE")
+    fi
+    cargo build --no-default-features "${additional_flags[@]}"
+    perf record --freq=2000 --call-graph dwarf --output=perf.data target/${PROFILE}/compare
+
+    # Convert to a format firefox will read
+    # flags to consider --show-info
+    perf script -F +pid --input perf.data > perf.firefox
+
+    echo "You probably want to go to https://profiler.firefox.com/"
+    echo "Either that or run hotspot"
+}
+
+main "${@}"

scripts/run_docker_compare.bash

@@ -0,0 +1,56 @@
+#!/usr/bin/env bash
+#
+set -euo pipefail
+IFS=$'\n\t'
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+: ${SHELL:="NO"} # or YES to launch a shell instead of running the test
+: ${TRACE:="NO"} # or YES to send traces to jaeger
+: ${BACKTRACE:="NO"} # or YES to print a rust backtrace when panicking
+: ${NO_COLOR:=""} # Set to anything to disable color output
+
+
+cd "$DIR/../"
+REALPATH=$(command -v uu-realpath || command -v realpath)
+MAKE=$(command -v gmake || command -v make)
+
+function main {
+    build_container
+    launch_container
+}
+
+function build_container {
+    $MAKE -C "$DIR/../docker/organic_test"
+}
+
+function launch_container {
+    local additional_flags=()
+    local additional_args=()
+    local features=(compare)
+
+    if [ "$NO_COLOR" != "" ]; then
+        additional_flags+=(--env "NO_COLOR=$NO_COLOR")
+    fi
+
+    if [ "$TRACE" = "YES" ]; then
+        # We use the host network so it can talk to jaeger hosted at 127.0.0.1
+        additional_flags+=(--network=host --env RUST_LOG=debug)
+        features+=(tracing)
+    fi
+
+    if [ "$SHELL" != "YES" ]; then
+        local features_joined=$(IFS=","; echo "${features[*]}")
+        additional_args+=(cargo run --no-default-features --features "$features_joined")
+    else
+        additional_args+=(/bin/sh)
+        additional_flags+=(-t)
+    fi
+
+    if [ "$BACKTRACE" = "YES" ]; then
+        additional_flags+=(--env RUST_BACKTRACE=full)
+    fi
+
+    docker run "${additional_flags[@]}" --init --rm -i -v "$($REALPATH ./):/source:ro" --mount source=cargo-cache,target=/usr/local/cargo/registry --mount source=rust-cache,target=/target --env CARGO_TARGET_DIR=/target -w /source --entrypoint "" organic-test "${additional_args[@]}"
+}
+
+main "${@}"

scripts/run_docker_integration_test.bash

@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+#
+set -euo pipefail
+IFS=$'\n\t'
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+: ${NO_COLOR:=""} # Set to anything to disable color output
+
+cd "$DIR/../"
+REALPATH=$(command -v uu-realpath || command -v realpath)
+MAKE=$(command -v gmake || command -v make)
+
+function main {
+    local test_names=$(get_test_names "${@}")
+    build_container
+
+    local test
+    while read test; do
+        launch_container "$test"
+    done<<<"$test_names"
+}
+
+function build_container {
+    $MAKE -C "$DIR/../docker/organic_test"
+}
+
+function get_test_names {
+    local test_file
+    local samples_dir=$($REALPATH "$DIR/../org_mode_samples")
+    for test_file in "$@"
+    do
+        if [ -e "$test_file" ]; then
+            local test_file_full_path=$($REALPATH "$test_file")
+            local relative_to_samples=$($REALPATH --relative-to "$samples_dir" "$test_file_full_path")
+            local without_extension="${relative_to_samples%.org}"
+            echo "${without_extension/\//_}" | tr '[:upper:]' '[:lower:]'
+        else
+            echo "$test_file" | tr '[:upper:]' '[:lower:]'
+        fi
+    done
+}
+
+function launch_container {
+    local test="$1"
+    local additional_flags=()
+
+    if [ "$NO_COLOR" != "" ]; then
+        additional_flags+=(--env "NO_COLOR=$NO_COLOR")
+    fi
+
+    local init_script=$(cat <<EOF
+set -euo pipefail
+IFS=\$'\n\t'
+
+cargo test --no-fail-fast --lib --test test_loader "$test" -- --show-output
+EOF
+          )
+
+    docker run "${additional_flags[@]}" --init --rm -v "$($REALPATH ./):/source:ro" --mount source=cargo-cache,target=/usr/local/cargo/registry --mount source=rust-cache,target=/target --env CARGO_TARGET_DIR=/target -w /source --entrypoint "" organic-test sh -c "$init_script"
+}
+
+
+main "${@}"

scripts/run_integration_test.bash

@@ -4,17 +4,27 @@ set -euo pipefail
 IFS=$'\n\t'
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 
+cd "$DIR/../"
 REALPATH=$(command -v uu-realpath || command -v realpath)
 
-samples_dir=$(readlink -f "$DIR/../org_mode_samples")
+function main {
+    local test_names=$(get_test_names "${@}")
+
+    local test
+    while read test; do
+        cargo test --no-fail-fast --test test_loader "$test" -- --show-output
+    done<<<"$test_names"
+}
 
 function get_test_names {
+    local test_file
+    local samples_dir=$($REALPATH "$DIR/../org_mode_samples")
     for test_file in "$@"
     do
         if [ -e "$test_file" ]; then
-            test_file_full_path=$(readlink -f "$test_file")
+            local test_file_full_path=$($REALPATH "$test_file")
-            relative_to_samples=$($REALPATH --relative-to "$samples_dir" "$test_file_full_path")
+            local relative_to_samples=$($REALPATH --relative-to "$samples_dir" "$test_file_full_path")
-            without_extension="${relative_to_samples%.org}"
+            local without_extension="${relative_to_samples%.org}"
             echo "${without_extension/\//_}" | tr '[:upper:]' '[:lower:]'
         else
             echo "$test_file" | tr '[:upper:]' '[:lower:]'
@@ -22,6 +32,4 @@ function get_test_names {
     done
 }
 
-get_test_names "$@" | while read test; do
+main "${@}"
-    (cd "$DIR/../" && cargo test --no-fail-fast --test test_loader "$test" -- --show-output)
-done

scripts/time_parse.bash

@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+#
+# Time running a single parse without invoking a compare with emacs.
+set -euo pipefail
+IFS=$'\n\t'
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+: ${PROFILE:="release-lto"}
+
+cd "$DIR/../"
+
+function main {
+    local additional_flags=()
+    if [ "$PROFILE" = "dev" ] || [ "$PROFILE" = "debug" ]; then
+        PROFILE="debug"
+    else
+        additional_flags+=(--profile "$PROFILE")
+    fi
+    cargo build --no-default-features "${additional_flags[@]}"
+    time ./target/${PROFILE}/compare
+}
+
+main "${@}"

src/compare/diff.rs

@@ -1,5 +1,8 @@
+use std::collections::HashSet;
+
 use super::util::assert_bounds;
 use super::util::assert_name;
+use crate::parser::sexp::unquote;
 use crate::parser::sexp::Token;
 use crate::parser::AngleLink;
 use crate::parser::Bold;
@@ -84,12 +87,24 @@ impl DiffResult {
             match self.status {
                 DiffStatus::Good => {
                     if self.has_bad_children() {
-                        "BADCHILD"
+                        format!(
+                            "{color}BADCHILD{reset}",
+                            color = DiffResult::foreground_color(255, 255, 0),
+                            reset = DiffResult::reset_color(),
+                        )
                     } else {
-                        "GOOD"
+                        format!(
+                            "{color}GOOD{reset}",
+                            color = DiffResult::foreground_color(0, 255, 0),
+                            reset = DiffResult::reset_color(),
+                        )
                     }
                 }
-                DiffStatus::Bad => "BAD",
+                DiffStatus::Bad => format!(
+                    "{color}BAD{reset}",
+                    color = DiffResult::foreground_color(255, 0, 0),
+                    reset = DiffResult::reset_color(),
+                ),
             }
         };
         println!(
@@ -117,6 +132,45 @@ impl DiffResult {
             DiffStatus::Bad => true,
         }
     }
+
+    fn foreground_color(red: u8, green: u8, blue: u8) -> String {
+        if DiffResult::should_use_color() {
+            format!(
+                "\x1b[38;2;{red};{green};{blue}m",
+                red = red,
+                green = green,
+                blue = blue
+            )
+        } else {
+            String::new()
+        }
+    }
+
+    #[allow(dead_code)]
+    fn background_color(red: u8, green: u8, blue: u8) -> String {
+        if DiffResult::should_use_color() {
+            format!(
+                "\x1b[48;2;{red};{green};{blue}m",
+                red = red,
+                green = green,
+                blue = blue
+            )
+        } else {
+            String::new()
+        }
+    }
+
+    fn reset_color() -> &'static str {
+        if DiffResult::should_use_color() {
+            "\x1b[0m"
+        } else {
+            ""
+        }
+    }
+
+    fn should_use_color() -> bool {
+        !std::env::var("NO_COLOR").is_ok_and(|val| !val.is_empty())
+    }
 }
 
 fn compare_element<'s>(
@@ -272,6 +326,7 @@ fn compare_heading<'s>(
     let children = emacs.as_list()?;
     let mut child_status = Vec::new();
     let mut this_status = DiffStatus::Good;
+    let mut message = None;
     let emacs_name = "headline";
     if assert_name(emacs, emacs_name).is_err() {
         this_status = DiffStatus::Bad;
@@ -281,6 +336,45 @@ fn compare_heading<'s>(
         this_status = DiffStatus::Bad;
     }
 
+    // Compare tags
+    let emacs_tags = get_tags_from_heading(emacs)?;
+    let emacs_tags: HashSet<_> = emacs_tags.iter().map(|val| val.as_str()).collect();
+    let rust_tags: HashSet<&str> = rust.tags.iter().map(|val| *val).collect();
+    let difference: Vec<&str> = emacs_tags
+        .symmetric_difference(&rust_tags)
+        .map(|val| *val)
+        .collect();
+    if !difference.is_empty() {
+        this_status = DiffStatus::Bad;
+        message = Some(format!("Mismatched tags: {}", difference.join(", ")));
+    }
+
+    // Compare todo-keyword
+    let todo_keyword = {
+        let children = emacs.as_list()?;
+        let attributes_child = children
+            .iter()
+            .nth(1)
+            .ok_or("Should have an attributes child.")?;
+        let attributes_map = attributes_child.as_map()?;
+        let todo_keyword = attributes_map
+            .get(":todo-keyword")
+            .ok_or("Missing :todo-keyword attribute.");
+        todo_keyword?.as_atom()?
+    };
+    match (todo_keyword, rust.todo_keyword, unquote(todo_keyword)) {
+        ("nil", None, _) => {}
+        (_, Some(rust_todo), Ok(emacs_todo)) if emacs_todo == rust_todo => {}
+        (emacs_todo, rust_todo, _) => {
+            this_status = DiffStatus::Bad;
+            message = Some(format!(
+                "(emacs != rust) {:?} != {:?}",
+                emacs_todo, rust_todo
+            ));
+        }
+    };
+
+    // Compare title
     let title = {
         let children = emacs.as_list()?;
         let attributes_child = children
@@ -297,6 +391,9 @@ fn compare_heading<'s>(
         child_status.push(compare_object(source, emacs_child, rust_child)?);
     }
 
+    // TODO: Compare todo-type, level, priority
+
+    // Compare section
     for (emacs_child, rust_child) in children.iter().skip(2).zip(rust.children.iter()) {
         match rust_child {
             DocumentElement::Heading(rust_heading) => {
@@ -311,11 +408,44 @@ fn compare_heading<'s>(
     Ok(DiffResult {
         status: this_status,
         name: emacs_name.to_owned(),
-        message: None,
+        message,
         children: child_status,
     })
 }
 
+fn get_tags_from_heading<'s>(
+    emacs: &'s Token<'s>,
+) -> Result<HashSet<String>, Box<dyn std::error::Error>> {
+    let children = emacs.as_list()?;
+    let attributes_child = children
+        .iter()
+        .nth(1)
+        .ok_or("Should have an attributes child.")?;
+    let attributes_map = attributes_child.as_map()?;
+    let tags = attributes_map
+        .get(":tags")
+        .ok_or("Missing :tags attribute.")?;
+    match tags.as_atom() {
+        Ok("nil") => {
+            return Ok(HashSet::new());
+        }
+        Ok(val) => panic!("Unexpected value for tags: {:?}", val),
+        Err(_) => {}
+    };
+    let tags = {
+        let tags = tags.as_list()?;
+        let strings = tags
+            .iter()
+            .map(Token::as_atom)
+            .collect::<Result<Vec<&str>, _>>()?;
+        strings
+            .into_iter()
+            .map(unquote)
+            .collect::<Result<HashSet<String>, _>>()?
+    };
+    Ok(tags)
+}
+
 fn compare_paragraph<'s>(
     source: &'s str,
     emacs: &'s Token<'s>,
@@ -974,7 +1104,7 @@ fn compare_plain_text<'s>(
             rust.source.len()
         ));
     }
-    let unquoted_text = text.unquote()?;
+    let unquoted_text = unquote(text.text)?;
     if unquoted_text != rust.source {
         this_status = DiffStatus::Bad;
         message = Some(format!(

src/compare/mod.rs

@@ -3,3 +3,5 @@ mod parse;
 mod util;
 pub use diff::compare_document;
 pub use parse::emacs_parse_org_document;
+pub use parse::get_emacs_version;
+pub use parse::get_org_mode_version;

src/compare/parse.rs

@@ -1,22 +1,25 @@
-use std::path::Path;
 use std::process::Command;
 
-pub fn emacs_parse_org_document<'a, C>(file_path: C) -> Result<String, Box<dyn std::error::Error>>
+pub fn emacs_parse_org_document<C>(file_contents: C) -> Result<String, Box<dyn std::error::Error>>
 where
-    C: AsRef<Path>,
+    C: AsRef<str>,
 {
-    let elisp_script = r#"(progn
+    let escaped_file_contents = escape_elisp_string(file_contents);
+    let elisp_script = format!(
+        r#"(progn
+     (erase-buffer)
+     (insert "{escaped_file_contents}")
      (org-mode)
      (message "%s" (pp-to-string (org-element-parse-buffer)))
-)"#;
+)"#,
+        escaped_file_contents = escaped_file_contents
+    );
     let mut cmd = Command::new("emacs");
     let proc = cmd
         .arg("-q")
         .arg("--no-site-file")
         .arg("--no-splash")
         .arg("--batch")
-        .arg("--insert")
-        .arg(file_path.as_ref().as_os_str())
         .arg("--eval")
         .arg(elisp_script);
     let out = proc.output()?;
@@ -24,3 +27,62 @@ where
     let org_sexp = out.stderr;
     Ok(String::from_utf8(org_sexp)?)
 }
+
+fn escape_elisp_string<C>(file_contents: C) -> String
+where
+    C: AsRef<str>,
+{
+    let source = file_contents.as_ref();
+    let source_len = source.len();
+    // We allocate a string 10% larger than the source to account for escape characters. Without this, we would have more allocations during processing.
+    let mut output = String::with_capacity(source_len + (source_len / 10));
+    for c in source.chars() {
+        match c {
+            '"' | '\\' => {
+                output.push('\\');
+                output.push(c);
+            }
+            _ => {
+                output.push(c);
+            }
+        }
+    }
+    output
+}
+
+pub fn get_emacs_version() -> Result<String, Box<dyn std::error::Error>> {
+    let elisp_script = r#"(progn
+     (message "%s" (version))
+)"#;
+    let mut cmd = Command::new("emacs");
+    let proc = cmd
+        .arg("-q")
+        .arg("--no-site-file")
+        .arg("--no-splash")
+        .arg("--batch")
+        .arg("--eval")
+        .arg(elisp_script);
+
+    let out = proc.output()?;
+    out.status.exit_ok()?;
+    Ok(String::from_utf8(out.stderr)?)
+}
+
+pub fn get_org_mode_version() -> Result<String, Box<dyn std::error::Error>> {
+    let elisp_script = r#"(progn
+     (org-mode)
+     (message "%s" (org-version nil t nil))
+)"#;
+    let mut cmd = Command::new("emacs");
+    let proc = cmd
+        .arg("-q")
+        .arg("--no-site-file")
+        .arg("--no-splash")
+        .arg("--batch")
+        .arg("--eval")
+        .arg(elisp_script);
+
+    let out = proc.output()?;
+    out.status.exit_ok()?;
+    Ok(String::from_utf8(out.stderr)?)
+}

src/compare/util.rs

@@ -42,24 +42,98 @@ pub fn assert_bounds<'s, S: Source<'s>>(
     emacs: &'s Token<'s>,
     rust: &'s S,
 ) -> Result<(), Box<dyn std::error::Error>> {
+    let standard_properties = get_standard_properties(emacs)?;
+    let (begin, end) = (
+        standard_properties
+            .begin
+            .ok_or("Token should have a begin.")?,
+        standard_properties.end.ok_or("Token should have an end.")?,
+    );
+    let (rust_begin, rust_end) = get_offsets(source, rust);
+    if (rust_begin + 1) != begin || (rust_end + 1) != end {
+        Err(format!("Rust bounds ({rust_begin}, {rust_end}) do not match emacs bounds ({emacs_begin}, {emacs_end})", rust_begin = rust_begin + 1, rust_end = rust_end + 1, emacs_begin=begin, emacs_end=end))?;
+    }
+
+    Ok(())
+}
+
+struct StandardProperties {
+    begin: Option<usize>,
+    #[allow(dead_code)]
+    post_affiliated: Option<usize>,
+    #[allow(dead_code)]
+    contents_begin: Option<usize>,
+    #[allow(dead_code)]
+    contents_end: Option<usize>,
+    end: Option<usize>,
+    #[allow(dead_code)]
+    post_blank: Option<usize>,
+}
+
+fn get_standard_properties<'s>(
+    emacs: &'s Token<'s>,
+) -> Result<StandardProperties, Box<dyn std::error::Error>> {
     let children = emacs.as_list()?;
     let attributes_child = children
         .iter()
         .nth(1)
         .ok_or("Should have an attributes child.")?;
     let attributes_map = attributes_child.as_map()?;
-    let begin = attributes_map
+    let standard_properties = attributes_map.get(":standard-properties");
-        .get(":begin")
+    Ok(if standard_properties.is_some() {
-        .ok_or("Missing :begin attribute.")?
+        let mut std_props = standard_properties
-        .as_atom()?;
+            .expect("if statement proves its Some")
-    let end = attributes_map
+            .as_vector()?
-        .get(":end")
+            .into_iter();
-        .ok_or("Missing :end attribute.")?
+        let begin = maybe_token_to_usize(std_props.next())?;
-        .as_atom()?;
+        let post_affiliated = maybe_token_to_usize(std_props.next())?;
-    let (rust_begin, rust_end) = get_offsets(source, rust);
+        let contents_begin = maybe_token_to_usize(std_props.next())?;
-    if (rust_begin + 1).to_string() != begin || (rust_end + 1).to_string() != end {
+        let contents_end = maybe_token_to_usize(std_props.next())?;
-        Err(format!("Rust bounds ({rust_begin}, {rust_end}) do not match emacs bounds ({emacs_begin}, {emacs_end})", rust_begin = rust_begin + 1, rust_end = rust_end + 1, emacs_begin=begin, emacs_end=end))?;
+        let end = maybe_token_to_usize(std_props.next())?;
+        let post_blank = maybe_token_to_usize(std_props.next())?;
+        StandardProperties {
+            begin,
+            post_affiliated,
+            contents_begin,
+            contents_end,
+            end,
+            post_blank,
         }
-
+    } else {
-    Ok(())
+        let begin = maybe_token_to_usize(attributes_map.get(":begin").map(|token| *token))?;
+        let end = maybe_token_to_usize(attributes_map.get(":end").map(|token| *token))?;
+        let contents_begin =
+            maybe_token_to_usize(attributes_map.get(":contents-begin").map(|token| *token))?;
+        let contents_end =
+            maybe_token_to_usize(attributes_map.get(":contents-end").map(|token| *token))?;
+        let post_blank =
+            maybe_token_to_usize(attributes_map.get(":post-blank").map(|token| *token))?;
+        let post_affiliated =
+            maybe_token_to_usize(attributes_map.get(":post-affiliated").map(|token| *token))?;
+        StandardProperties {
+            begin,
+            post_affiliated,
+            contents_begin,
+            contents_end,
+            end,
+            post_blank,
+        }
+    })
+}
+
+fn maybe_token_to_usize(
+    token: Option<&Token<'_>>,
+) -> Result<Option<usize>, Box<dyn std::error::Error>> {
+    Ok(token
+        .map(|token| token.as_atom())
+        .map_or(Ok(None), |r| r.map(Some))?
+        .map(|val| {
+            if val == "nil" {
+                None
+            } else {
+                Some(val.parse::<usize>())
+            }
+        })
+        .flatten() // Outer option is whether or not the param exists, inner option is whether or not it is nil
+        .map_or(Ok(None), |r| r.map(Some))?)
 }

src/error/error.rs

@@ -4,6 +4,7 @@ use nom::IResult;
 
 pub type Res<T, U> = IResult<T, U, CustomError<T>>;
 
+// TODO: MyError probably shouldn't be based on the same type as the input type since it's used exclusively with static strings right now.
 #[derive(Debug, PartialEq)]
 pub enum CustomError<I> {
     MyError(MyError<I>),