Skip to content

resolve #8 add CloseMarkdownDrawer#9

Open
Scuilion wants to merge 2 commits intomasterfrom
feature/close-function-#8
Open

resolve #8 add CloseMarkdownDrawer#9
Scuilion wants to merge 2 commits intomasterfrom
feature/close-function-#8

Conversation

@Scuilion
Copy link
Copy Markdown
Owner

No description provided.

@Scuilion
Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist
Copy link
Copy Markdown

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@Scuilion
Copy link
Copy Markdown
Owner Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request adds a function to close the markdown drawer and a corresponding command. The functionality seems straightforward and useful. However, there's a minor naming inconsistency that should be addressed.

Summary of Findings

  • Command Naming Consistency: The command MarkDrawerClose in ftplugin/markdown.vim calls a function named CloseMarkdownClose, which seems inconsistent. It should call CloseMarkdownDrawer for clarity and consistency.

Assessment

The pull request introduces a new command to close the Markdown drawer, which enhances the user experience. The changes are relatively small, but I've identified a potential issue with the command name in ftplugin/markdown.vim. I recommend addressing this comment before merging. Please have others review and approve this code before merging.

Comment thread ftplugin/markdown.vim
endif

command! MarkDrawer :call ui#OpenMarkdownDrawer()
command! MarkDrawerClose :call ui#CloseMarkdownClose()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The command MarkDrawerClose calls CloseMarkdownClose, but the function in ui.vim is named CloseMarkdownDrawer. For consistency, should this call ui#CloseMarkdownDrawer() instead?

command! MarkDrawerClose :call ui#CloseMarkdownDrawer()

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.

1 participant