Expand tests, remove option to change YAML parser (#431)

* Expand tests

* Remove option to change parser

README.md

@@ -473,14 +473,6 @@ This is a complete list of all of the options that can be passed to `require("ob
       return string.format("![%s](%s)", display_name, link_path)
     end,
   },
-
-  -- Optional, set the YAML parser to use. The valid options are:
-  --  * "native" - uses a pure Lua parser that's fast but potentially misses some edge cases.
-  --  * "yq" - uses the command-line tool yq (https://github.com/mikefarah/yq), which is more robust
-  --    but much slower and needs to be installed separately.
-  -- In general you should be using the native parser unless you run into a bug with it, in which
-  -- case you can temporarily switch to the "yq" parser until the bug is fixed.
-  yaml_parser = "native",
 }
 ```
 

lua/obsidian/client.lua

@@ -93,11 +93,6 @@ Client.new = function(opts)
 
   self:set_workspace(workspace)
 
-  if self.opts.yaml_parser ~= nil then
-    local yaml = require "obsidian.yaml"
-    yaml.set_parser(self.opts.yaml_parser)
-  end
-
   return self
 end
 

lua/obsidian/config.lua

@@ -29,7 +29,6 @@ local config = {}
 ---@field open_notes_in obsidian.config.OpenStrategy
 ---@field ui obsidian.config.UIOpts
 ---@field attachments obsidian.config.AttachmentsOpts
----@field yaml_parser string|?
 config.ClientOpts = {}
 
 --- Get defaults.
@@ -61,7 +60,6 @@ config.ClientOpts.default = function()
     open_notes_in = "current",
     ui = config.UIOpts.default(),
     attachments = config.AttachmentsOpts.default(),
-    yaml_parser = "native",
   }
 end
 

lua/obsidian/path.lua

@@ -96,8 +96,8 @@ local function get_stem(path)
   end
 end
 
---- A Path class that provides a subset of the functionality of the Python pathlib library while
---- staying true to its API. It improves on a number of bugs in plenary.path.
+--- A `Path` class that provides a subset of the functionality of the Python `pathlib` library while
+--- staying true to its API. It improves on a number of bugs in `plenary.path`.
 ---
 ---@toc_entry obsidian.Path
 ---
@@ -179,12 +179,18 @@ end
 
 --- Get a temporary path with a unique name.
 ---
+---@param opts { suffix: string|? }|?
+---
 ---@return obsidian.Path
-Path.temp = function()
+Path.temp = function(opts)
+  opts = opts or {}
   -- os.tmpname gives us a temporary file, but we really want a temporary directory, so we
   -- immediately delete that file.
   local tmpname = os.tmpname()
   os.remove(tmpname)
+  if opts.suffix then
+    tmpname = tmpname .. opts.suffix
+  end
   return Path.new(tmpname)
 end
 
@@ -441,7 +447,7 @@ Path.mkdir = function(self, opts)
   self:mkdir { mode = mode }
 end
 
---- Remove the corresponding directory.
+--- Remove the corresponding directory. This directory must be empty.
 Path.rmdir = function(self)
   local resolved = self:resolve { strict = false }
 
@@ -455,6 +461,33 @@ Path.rmdir = function(self)
   end
 end
 
+--- Recursively remove an entire directory and its contents.
+Path.rmtree = function(self)
+  local scan = require "plenary.scandir"
+
+  local resolved = self:resolve { strict = true }
+  if not resolved:is_dir() then
+    error("NotADirectoryError: " .. resolved.filename)
+  end
+
+  -- First unlink all files.
+  scan.scan_dir(resolved.filename, {
+    hidden = true,
+    on_insert = function(file)
+      Path.new(file):unlink()
+    end,
+  })
+
+  -- Now iterate backwards to clean up remaining dirs.
+  local dirs = scan.scan_dir(resolved.filename, { add_dirs = true, hidden = true })
+  for i = #dirs, 1, -1 do
+    Path.new(dirs[i]):rmdir()
+  end
+
+  -- And finally remove the top level dir.
+  resolved:rmdir()
+end
+
 --- Create a file at this given path.
 ---
 ---@param opts { mode: integer|?, exist_ok: boolean|? }|?

lua/obsidian/yaml/init.lua

@@ -1,34 +1,13 @@
 local util = require "obsidian.util"
+local parser = require "obsidian.yaml.parser"
 
 local yaml = {}
 
-yaml.parsers = {
-  ["native"] = require "obsidian.yaml.native",
-  ["yq"] = require "obsidian.yaml.yq",
-}
-
-yaml.parser = "native"
-
----Set the YAML parser to use.
----@param parser string
-yaml.set_parser = function(parser)
-  if yaml.parsers[parser] == nil then
-    error("Undefined parser " .. parser)
-  else
-    yaml.parser = parser
-  end
-end
-
----Reset to the default parser.
-yaml.reset_parser = function()
-  yaml.parser = "native"
-end
-
 ---Deserialize a YAML string.
 ---@param str string
 ---@return any
 yaml.loads = function(str)
-  return yaml.parsers[yaml.parser].loads(str)
+  return parser.loads(str)
 end
 
 ---@param s string

test/obsidian/client_spec.lua

@@ -4,14 +4,9 @@ local obsidian = require "obsidian"
 
 ---Get a client in a temporary directory.
 ---
----@return obsidian.Client
-local tmp_client = function()
-  -- This gives us a tmp file name, but we really want a directory.
-  -- So we delete that file immediately.
-  local tmpname = os.tmpname()
-  os.remove(tmpname)
-
-  local dir = Path:new(tmpname .. "-obsidian/")
+---@param run fun(client: obsidian.Client)
+local with_tmp_client = function(run)
+  local dir = Path.temp { suffix = "-obsidian" }
   dir:mkdir { parents = true }
 
   local client = obsidian.new_from_dir(tostring(dir))
@@ -27,93 +22,110 @@ local tmp_client = function()
     return id
   end
 
-  return client
+  local ok, err = pcall(run, client)
+
+  dir:rmtree()
+
+  if not ok then
+    error(err)
+  end
 end
 
 describe("Client", function()
   it("should be able to initialize a daily note", function()
-    local client = tmp_client()
-    local note = client:today()
-    assert.is_true(note.path ~= nil)
-    assert.is_true(note:exists())
+    with_tmp_client(function(client)
+      local note = client:today()
+      assert.is_true(note.path ~= nil)
+      assert.is_true(note:exists())
+    end)
   end)
 
   it("should not add frontmatter for today when disabled", function()
-    local client = tmp_client()
-    client.opts.disable_frontmatter = true
-    local new_note = client:today()
+    with_tmp_client(function(client)
+      client.opts.disable_frontmatter = true
+      local new_note = client:today()
 
-    local saved_note = Note.from_file(new_note.path)
-    assert.is_false(saved_note.has_frontmatter)
+      local saved_note = Note.from_file(new_note.path)
+      assert.is_false(saved_note.has_frontmatter)
+    end)
   end)
 
   it("should not add frontmatter for yesterday when disabled", function()
-    local client = tmp_client()
-    client.opts.disable_frontmatter = true
-    local new_note = client:yesterday()
+    with_tmp_client(function(client)
+      client.opts.disable_frontmatter = true
+      local new_note = client:yesterday()
 
-    local saved_note = Note.from_file(new_note.path)
-    assert.is_false(saved_note.has_frontmatter)
+      local saved_note = Note.from_file(new_note.path)
+      assert.is_false(saved_note.has_frontmatter)
+    end)
   end)
 
   it("should parse a title that's a partial path and generate new ID", function()
-    local client = tmp_client()
-    local title, id, path = client:parse_title_id_path "notes/Foo"
-    assert.equals(title, "Foo")
-    assert.equals(id, "foo")
-    assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "foo.md"))
+    with_tmp_client(function(client)
+      local title, id, path = client:parse_title_id_path "notes/Foo"
+      assert.equals(title, "Foo")
+      assert.equals(id, "foo")
+      assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "foo.md"))
+    end)
   end)
 
   it("should parse an ID that's a path", function()
-    local client = tmp_client()
-    local title, id, path = client:parse_title_id_path("Foo", "notes/1234-foo")
-    assert.equals(title, "Foo")
-    assert.equals(id, "1234-foo")
-    assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "1234-foo.md"))
+    with_tmp_client(function(client)
+      local title, id, path = client:parse_title_id_path("Foo", "notes/1234-foo")
+      assert.equals(title, "Foo")
+      assert.equals(id, "1234-foo")
+      assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "1234-foo.md"))
+    end)
   end)
 
   it("should parse a title that's an exact path", function()
-    local client = tmp_client()
-    local title, id, path = client:parse_title_id_path "notes/foo.md"
-    assert.equals(title, "foo")
-    assert.equals(id, "foo")
-    assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "foo.md"))
+    with_tmp_client(function(client)
+      local title, id, path = client:parse_title_id_path "notes/foo.md"
+      assert.equals(title, "foo")
+      assert.equals(id, "foo")
+      assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "foo.md"))
+    end)
   end)
 
   it("should ignore boundary whitespace when parsing a title", function()
-    local client = tmp_client()
-    local title, id, path = client:parse_title_id_path "notes/Foo  "
-    assert.equals(title, "Foo")
-    assert.equals(id, "foo")
-    assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "foo.md"))
+    with_tmp_client(function(client)
+      local title, id, path = client:parse_title_id_path "notes/Foo  "
+      assert.equals(title, "Foo")
+      assert.equals(id, "foo")
+      assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "foo.md"))
+    end)
   end)
 
   it("should keep whitespace within a path when parsing a title", function()
-    local client = tmp_client()
-    local title, id, path = client:parse_title_id_path "notes/Foo Bar.md"
-    assert.equals(title, "Foo Bar")
-    assert.equals(id, "Foo Bar")
-    assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "Foo Bar.md"))
+    with_tmp_client(function(client)
+      local title, id, path = client:parse_title_id_path "notes/Foo Bar.md"
+      assert.equals(title, "Foo Bar")
+      assert.equals(id, "Foo Bar")
+      assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / "Foo Bar.md"))
+    end)
   end)
 
   it("should generate a new id when the title is just a folder", function()
-    local client = tmp_client()
-    local title, id, path = client:parse_title_id_path "notes/"
-    assert.equals(title, nil)
-    assert.equals(#id, 4)
-    assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / (id .. ".md")))
+    with_tmp_client(function(client)
+      local title, id, path = client:parse_title_id_path "notes/"
+      assert.equals(title, nil)
+      assert.equals(#id, 4)
+      assert.equals(tostring(path), tostring(Path:new(client.dir) / "notes" / (id .. ".md")))
+    end)
   end)
 
   it("should prepare search opts properly", function()
-    local client = tmp_client()
-    ---@diagnostic disable-next-line: invisible
-    local opts = client:_prepare_search_opts(true, { max_count_per_file = 1 })
-    assert.are_same(opts:to_ripgrep_opts(), { "--sortr=modified", "-m=1" })
+    with_tmp_client(function(client)
+      ---@diagnostic disable-next-line: invisible
+      local opts = client:_prepare_search_opts(true, { max_count_per_file = 1 })
+      assert.are_same(opts:to_ripgrep_opts(), { "--sortr=modified", "-m=1" })
+    end)
   end)
 
   it("should resolve relative paths", function()
-    local client = tmp_client()
-    assert.are_same(client:vault_relative_path "foo.md", Path.new "foo.md")
-    assert.are_same(client:vault_relative_path(client.dir / "foo.md"), Path.new "foo.md")
+    with_tmp_client(function(client)
+      assert.are_same(client:vault_relative_path "foo.md", Path.new "foo.md")
+      assert.are_same(client:vault_relative_path(client.dir / "foo.md"), Path.new "foo.md")
+    end)
   end)
 end)

test/obsidian/yaml/parser_spec.lua

@@ -1,4 +1,4 @@
-local yaml = require "obsidian.yaml.native"
+local yaml = require "obsidian.yaml.parser"
 local util = require "obsidian.util"
 
 describe("Parser class", function()