Skip to content
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

Issue 632 support for obsidian callouts #665

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

aquilesg
Copy link

This adds UI support for the callout groups that are defined
in the Obsidian documentation.

It also supports custom alias definitions.

The result is below:
Screenshot 2024-07-25 at 7 01 11 PM

@aquilesg
Copy link
Author

Looking at it makes me wonder if it's cleaner to not have the blue bit before the highlight group

Copy link
Owner

@epwalsh epwalsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @aquilesg this is a great start and I'm really excited about this feature. I ran into some bugs when I tried it out though:

When I have this:

>[!info] foo

it appears to work, although I think the space between ">" and "[" should be required.

image

When I do add the space:

> [!info] foo

I don't see the info icon anymore.

image

And when I add another line the first non-space character gets swallowed regardless of whether or not I have a space after ">":

> [!info] foo
> bar

image

@@ -515,14 +691,30 @@ local function update_extmarks(bufnr, ns_id, ui_opts)

-- Check if inside a code block or at code block boundary. If not, update marks.
if string.match(line, "^%s*```[^`]*$") then
inside_code_block = not inside_code_block
inside_code_block = true
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh whoops, I didn't mean to add this. Let me revert.

Comment on lines 1335 to 1368
-------------------------
--- Stack implementation
-------------------------

-- Create new stack
function util.new_stack()
return {}
end

-- Push an item onto the stack
function util.push(stack, item)
table.insert(stack, item)
end

-- Pop an item from the stack
function util.pop(stack)
if #stack == 0 then
return nil
end
return table.remove(stack)
end

-- Check the top item on the stack
function util.peek(stack)
if #stack == 0 then
return nil
end
return stack[#stack]
end

-- Check if the stack is empty
function util.is_empty(stack)
return #stack == 0
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this to lua/obsidian/collections.lua

Comment on lines 700 to 710
local count = 0
for c = 1, #line do
local char = line:sub(c, c)
if char == ">" then
count = count + 1
elseif char == "[" then
if line:sub(c, c + 2) == "[!]" then
break
end
end
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is just counting the number of ">"s then you could do that with regular expressions

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a n00b with Lua and regex so I'm struggling to find the correct way to do this.
Do you have any tips?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could just do local count = util.string_count(line, "> ")

@@ -36,6 +36,7 @@ M.Patterns = {

-- Miscellaneous
Highlight = "==[^=]+==", -- ==text==
Callout = "%[!.*%]",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't callouts always start with ">"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's right

@aquilesg aquilesg force-pushed the ISSUE-632-support-for-obsidian-callouts branch 3 times, most recently from 1f32e4e to 8befa8d Compare July 30, 2024 12:56
@aquilesg
Copy link
Author

I think I fixed most issues.
Screenshot 2024-07-30 at 8 04 08 AM

@aquilesg aquilesg requested a review from epwalsh July 30, 2024 22:07
Comment on lines 98 to 127
-- Create new stack
function M.new_stack()
return {}
end

-- Push an item onto the stack
function M.push(stack, item)
table.insert(stack, item)
end

-- Pop an item from the stack
function M.pop(stack)
if #stack == 0 then
return nil
end
return table.remove(stack)
end

-- Check the top item on the stack
function M.peek(stack)
if #stack == 0 then
return nil
end
return stack[#stack]
end

-- Check if the stack is empty
function M.is_empty(stack)
return #stack == 0
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really how Lua classes are implemented, and it doesn't seem necessary to have a separate class here anyway since it's just a very thin wrapper around a raw table/list.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with removing this all together then.

Comment on lines 488 to 489
callout_mark_start,
callout_mark_end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these args are actually used

Comment on lines 700 to 710
local count = 0
for c = 1, #line do
local char = line:sub(c, c)
if char == ">" then
count = count + 1
elseif char == "[" then
if line:sub(c, c + 2) == "[!]" then
break
end
end
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could just do local count = util.string_count(line, "> ")

@aquilesg aquilesg force-pushed the ISSUE-632-support-for-obsidian-callouts branch from c516b2a to f8538c4 Compare July 31, 2024 02:24
@aquilesg aquilesg requested a review from epwalsh August 6, 2024 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants