-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support workspaces config option #155
Changes from all commits
41cfd7f
070ef9f
2aaf7ff
d3313a3
ee7a3f7
9c10132
731fd49
8d71474
c5a3a8f
170dddb
9c19046
591a116
d780e10
acd5f23
a5931ba
c31a654
d3a93c5
417eb91
c741398
46b4a83
598bc0a
48ae1f1
e190304
1e1ba52
04dd838
852895f
79118e1
f422f98
9402d59
bc228a0
242fdbe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to keep backwards compatibility for when people upgrade. This should be fairly easy: Just check if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in f422f98 |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -10,8 +10,10 @@ obsidian.completion = require "obsidian.completion" | |||||
obsidian.note = require "obsidian.note" | ||||||
obsidian.util = require "obsidian.util" | ||||||
obsidian.mapping = require "obsidian.mapping" | ||||||
obsidian.workspace = require "obsidian.workspace" | ||||||
|
||||||
---@class obsidian.Client | ||||||
---@field current_workspace obsidian.Workspace | ||||||
---@field dir Path | ||||||
---@field templates_dir Path|? | ||||||
---@field opts obsidian.config.ClientOpts | ||||||
|
@@ -24,7 +26,10 @@ local client = {} | |||||
---@return obsidian.Client | ||||||
obsidian.new = function(opts) | ||||||
local self = setmetatable({}, { __index = client }) | ||||||
self.dir = Path:new(vim.fs.normalize(tostring(opts.dir and opts.dir or "./"))) | ||||||
|
||||||
self.current_workspace = obsidian.workspace.get_from_opts(opts) | ||||||
-- NOTE: workspace.path has already been normalized | ||||||
self.dir = Path:new(self.current_workspace.path) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should still normalize
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi! I finally have time to swing back around on these... The workspace paths should already be normalized. Here, in workspace.new = function(name, path)
local self = setmetatable({}, { __index = workspace })
self.name = name
self.path = vim.fs.normalize(path)
return self
end Paths are also normalized when reading from the config in for key, value in pairs(opts.workspaces) do
opts.workspaces[key].path = vim.fs.normalize(tostring(value.path))
end There could be an edge-case I haven't fully considered, but I think it's safe to omit the redundant path normalization. However, if you still prefer normalizing the paths where you suggested I won't argue with you 😄 |
||||||
self.opts = opts | ||||||
self.backlinks_namespace = vim.api.nvim_create_namespace "ObsidianBacklinks" | ||||||
|
||||||
|
@@ -37,7 +42,7 @@ end | |||||
---@return obsidian.Client | ||||||
obsidian.new_from_dir = function(dir) | ||||||
local opts = config.ClientOpts.default() | ||||||
opts.dir = vim.fs.normalize(dir) | ||||||
opts.workspaces = vim.tbl_extend("force", { obsidian.workspace.new_from_dir(dir) }, opts.workspaces) | ||||||
return obsidian.new(opts) | ||||||
end | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
---@class obsidian.Workspace | ||
---@field name string | ||
---@field path string | ||
local workspace = {} | ||
|
||
---Create a new workspace | ||
--- | ||
---@param name string Workspace name | ||
---@param path string Workspace path (will be normalized) | ||
--- | ||
---@return obsidian.Workspace | ||
workspace.new = function(name, path) | ||
local self = setmetatable({}, { __index = workspace }) | ||
|
||
self.name = name | ||
self.path = vim.fs.normalize(path) | ||
|
||
return self | ||
end | ||
|
||
workspace.new_from_cwd = function() | ||
return workspace.new_from_dir(vim.fn.getcwd()) | ||
end | ||
|
||
workspace.new_from_dir = function(dir) | ||
return workspace.new(vim.fn.fnamemodify(dir, ":t"), dir) | ||
end | ||
|
||
---Determines if cwd is a workspace | ||
--- | ||
---@param workspaces table<obsidian.Workspace> | ||
---@return obsidian.Workspace|nil | ||
workspace.get_workspace_from_cwd = function(workspaces) | ||
local cwd = vim.fn.getcwd() | ||
local _, value = next(vim.tbl_filter(function(w) | ||
if w.path == cwd then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might need to normalize these paths before comparing them, otherwise it seems a bit brittle. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my comment in a previous suggestion |
||
return true | ||
end | ||
return false | ||
end, workspaces)) | ||
|
||
return value | ||
end | ||
|
||
---Returns the default workspace | ||
--- | ||
---@param workspaces table<obsidian.Workspace> | ||
---@return obsidian.Workspace|nil | ||
workspace.get_default_workspace = function(workspaces) | ||
local _, value = next(workspaces) | ||
return value | ||
end | ||
|
||
---Resolves current workspace from client config | ||
--- | ||
---@param opts obsidian.config.ClientOpts | ||
---@return obsidian.Workspace | ||
workspace.get_from_opts = function(opts) | ||
local current_workspace | ||
|
||
if opts.detect_cwd then | ||
current_workspace = workspace.get_workspace_from_cwd(opts.workspaces) | ||
else | ||
current_workspace = workspace.get_default_workspace(opts.workspaces) | ||
end | ||
|
||
if not current_workspace then | ||
current_workspace = workspace.new_from_cwd() | ||
end | ||
|
||
return current_workspace | ||
end | ||
|
||
return workspace |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
local workspace = require "obsidian.workspace" | ||
|
||
local opts = { | ||
workspaces = { | ||
{ | ||
name = "work", | ||
path = "~/notes/work", | ||
}, | ||
{ | ||
name = "personal", | ||
path = "~/notes/personal", | ||
}, | ||
{ | ||
name = "cwd_workspace", | ||
path = os.getenv "PWD", | ||
}, | ||
}, | ||
detect_cwd = false, | ||
} | ||
|
||
describe("Workspace", function() | ||
it("should be able to initialize a workspace", function() | ||
local ws = workspace.new("test_workspace", "/tmp/obsidian_test_workspace") | ||
assert.equals("test_workspace", ws.name) | ||
assert.equals("/tmp/obsidian_test_workspace", ws.path) | ||
end) | ||
|
||
it("should be able to initialize from cwd", function() | ||
local ws = workspace.new_from_cwd() | ||
local cwd = os.getenv "PWD" | ||
assert.equals(vim.fn.fnamemodify(vim.fn.getcwd(), ":t"), ws.name) | ||
assert.equals(cwd, ws.path) | ||
end) | ||
|
||
it("should be able to retrieve the default workspace", function() | ||
local ws = workspace.get_default_workspace(opts.workspaces) | ||
assert.is_not(ws, nil) | ||
assert.equals(opts.workspaces[1].name, ws.name) | ||
assert.equals(opts.workspaces[1].path, ws.path) | ||
end) | ||
|
||
it("should initialize workspace from cwd", function() | ||
local ws = workspace.get_workspace_from_cwd(opts.workspaces) | ||
assert.equals(opts.workspaces[3].name, ws.name) | ||
assert.equals(opts.workspaces[3].path, ws.path) | ||
end) | ||
|
||
it("should return cwd workspace when detect_cwd is true", function() | ||
local old_cwd = opts.detect_cwd | ||
opts.detect_cwd = true | ||
local ws = workspace.get_from_opts(opts) | ||
assert.equals(opts.workspaces[3].name, ws.name) | ||
assert.equals(opts.workspaces[3].path, ws.path) | ||
opts.detect_cwd = old_cwd | ||
end) | ||
it("should return default workspace when detect_cwd is false", function() | ||
local old_cwd = opts.detect_cwd | ||
opts.detect_cwd = false | ||
local ws = workspace.get_from_opts(opts) | ||
assert.equals(opts.workspaces[1].name, ws.name) | ||
assert.equals(opts.workspaces[1].path, ws.path) | ||
opts.detect_cwd = old_cwd | ||
end) | ||
end) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same suggestion here about normalizing:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment in a previous suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok fair enough! Could you add a comment here at least:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with 598bc0a