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

feat: add FormatToggle keymap (#750) #751

Merged
merged 2 commits into from
May 18, 2023
Merged

feat: add FormatToggle keymap (#750) #751

merged 2 commits into from
May 18, 2023

Conversation

Groveer
Copy link
Contributor

@Groveer Groveer commented May 18, 2023

No description provided.

@CharlesChiuGit
Copy link
Collaborator

Functionally this should works, but I'm not sure how to feel about this.

@Groveer
Copy link
Contributor Author

Groveer commented May 18, 2023

I'm a C/C++ developer, I do't need format on save by default, it will modify my source file all. so, I set format on save to false, but sometimes when I edit html or md file, I need it auto format, so, I need a keymap to toggle it.

@CharlesChiuGit
Copy link
Collaborator

CharlesChiuGit commented May 18, 2023

have u consider just put your project path in the disable formatting settings? check lua\settings.lua

image

i think this is just simpler.

@CharlesChiuGit
Copy link
Collaborator

CharlesChiuGit commented May 18, 2023

i don't think everything needs to be send to upstreams. in fact, most of the maintainers of this repo keeps their own forks/repos to cater their needs.

u can fork this repo, create another branch, and cherry-pick the commits then merge into your own branch.

@CharlesChiuGit
Copy link
Collaborator

CharlesChiuGit commented May 18, 2023

i'm leaning towards to close this pr.

@Groveer
Copy link
Contributor Author

Groveer commented May 18, 2023

This settings means that every project I need to disable auto format must add it‘s directory here, which is far less convenient than keymap.

@Groveer
Copy link
Contributor Author

Groveer commented May 18, 2023

i don't think everything needs to be send to upstreams. in fact, most of the maintainers of this repo keeps their own forks/repos to cater their needs.

u can fork this repo, create another branch, and cherry-pick the commits then merge into your own branch.

I agree with you, but I think this feature is more user-friendly.

@CharlesChiuGit
Copy link
Collaborator

CharlesChiuGit commented May 18, 2023

This settings means that every project I need to disable auto format must add it‘s directory here, which is far less convenient than keymap.

Personally I just put all my projects in a folder called Workspace. If I were u, I could just add ~/Workspace to the list.

But I'm fine with new keymaps.

Copy link
Owner

@ayamir ayamir left a comment

Choose a reason for hiding this comment

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

Set a keymap for FormatToggle is fine, but why need to modify formatting.lua?

@Groveer
Copy link
Contributor Author

Groveer commented May 18, 2023

Because this is a bug, when I toggle to disable, nvim will not send a notification(toggle to enable wiil do this). modify formatting.lua will fix it.

@ayamir
Copy link
Owner

ayamir commented May 18, 2023

Because this is a bug, when I toggle to disable, nvim will not send a notification(toggle to enable wiil do this). modify formatting.lua will fix it.

Just add keymap works for me, never bug encountered.

@Jint-lzxy
Copy link
Collaborator

Because this is a bug, when I toggle to disable, nvim will not send a notification(toggle to enable wiil do this). modify formatting.lua will fix it.

In fact this is intentional. If one disables a config entry (such as format_on_save) in settings.lua, we would assume he has blocked that feature (just like supplying -U<feature_macro_name> during compilation). We do not expect users to use that feature in any way in the future, so there is no guarantee for any related behaviors.

@Jint-lzxy
Copy link
Collaborator

I'm also leaning towards closing this pr -

I'm a C/C++ developer, I do't need format on save by default, it will modify my source file all. so, I set format on save to false, but sometimes when I edit html or md file, I need it auto format, so, I need a keymap to toggle it.

u might be interested in Clang-Format Style Options, and if you really want to disable code formatting for c/cpp, you can remove the corresponding formatter(s):

btns.formatting.clang_format.with({
filetypes = { "c", "cpp" },
extra_args = require("completion.formatters.clang_format"),
}),

FWIW adding too many keymaps that are "seemingly" rare can make the config too cumbersome and further limit customizability. For features that have strong practicality for specific groups of users, they should be left to the users to implement those features themselves, which is also one of the reasons why we use neovim 😄

@Groveer
Copy link
Contributor Author

Groveer commented May 18, 2023

Because this is a bug, when I toggle to disable, nvim will not send a notification(toggle to enable wiil do this). modify formatting.lua will fix it.

In fact this is intentional. If one disables a config entry (such as format_on_save) in settings.lua, we would assume he has blocked that feature (just like supplying -U<feature_macro_name> during compilation). We do not expect users to use that feature in any way in the future, so there is no guarantee for any related behaviors.

Thank you for explaining, it's up to you to merge the PR or not.

@Jint-lzxy Jint-lzxy requested a review from ayamir May 18, 2023 13:19
local map_callback = bind.map_callback

local plug_map = {
-- LSP-related keymaps, work only when event = { "InsertEnter", "LspStart" }
Copy link
Owner

Choose a reason for hiding this comment

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

Format functionality is related with formatter in fact, some lsp server support it but others not. So this note should be remove.

@ayamir ayamir merged commit 41262de into ayamir:main May 18, 2023
@Groveer Groveer deleted the format branch May 19, 2023 02:51
singlemancombat pushed a commit to singlemancombat/nvim-config that referenced this pull request May 19, 2023
* feat: add `FormatToggle` keymap (ayamir#750)

* remove reduntant note.

---------

Co-authored-by: ayamir <[email protected]>
boomker pushed a commit to boomker/uvcode-nvim that referenced this pull request May 24, 2023
* feat: add `FormatToggle` keymap (ayamir#750)

* remove reduntant note.

---------

Co-authored-by: ayamir <[email protected]>
(cherry picked from commit 41262de)
bleedingfight pushed a commit to bleedingfight/nvimdots that referenced this pull request Jun 22, 2023
* feat: add `FormatToggle` keymap (ayamir#750)

* remove reduntant note.

---------

Co-authored-by: ayamir <[email protected]>
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.

4 participants