Make conform respect config.vim.lsp.formatOnSave and config.vim.lsp.mappings.toggleFormatOnSave by venkyr77 · Pull Request #768 · NotAShelf/nvf

venkyr77 · 2025-03-31T21:19:25Z

Update of PR #764

The current defaults(link1, link2)from conform config is not respecting the top level config we have for formatOnSave - vim.lsp.formatOnSave. Also, config.vim.lsp.mappings.toggleFormatOnSave has no impact.

From conform docs

-- If this is set, Conform will run the formatter on save.
-- It will pass the table to conform.format().
-- This can also be a function that returns the table

-- If this is set, Conform will run the formatter asynchronously after save.
-- It will pass the table to conform.format().
-- This can also be a function that returns the table.

these need to be set to null (or nil in lua) when config.vim.lsp.formatOnSave is false

This PR fixes conform config to respect config.vim.lsp.formatOnSave

Testing:

Tested my changes by changing the nvf url to the branch from my fork

nvf.url = "github:venkyr77/nvf/conform-format-on-save-fix";

with config.vim.lsp.formatOnSave = false; conform is not auto formatting the file

with config.vim.lsp.formatOnSave = true;

with config.vim.lsp.formatOnSave = true; and using the mapping for config.vim.lsp.mappings.toggleFormatOnSave

Sanity Checking

Add a 👍 reaction to pull requests you find important.

github-actions · 2025-03-31T21:20:18Z

🚀 Live preview deployed from `79993c7`

View it here:

Debug Information

Triggered by: horriblename

HEAD at: conform-format-on-save-fix

Reruns: 848

venkyr77 · 2025-03-31T21:20:19Z

@horriblename, fixes #764 (comment)

venkyr77 · 2025-03-31T21:25:33Z

+                else
+                  return ${toLuaObject defaultFormatAfterSaveOpts}


nit: no else return

venkyr77 · 2025-03-31T21:26:04Z

+                else
+                  return ${toLuaObject defaultFormatOnSaveOpts}


nit: no else return

if the changes look good, I can make a fast followup

horriblename

Looks good other than some documentation stuff

horriblename · 2025-04-02T17:55:55Z

+      in
+        mkOption {
+          type = nullOr (either attrs luaInline);
+          default =


add a defaultText that says something like "enabled with LSP format as fallback, respecting vim.lsp.formatOnSave and vim.lsp.mappings.toggleFormatOnSave"

also: use lib.options.literalMD in the defaultText

NotAShelf · 2025-04-02T18:17:25Z

+          type = nullOr (either attrs luaInline);
+          default =
+            mkLuaInline
+            # lua


Suggested change

# lua

Please don't do this. I understand it's easier to handle the code with syntax highlighting, but we don't inject syntax anywhere else in the source tree and Alejandra results in horrible formatting when you specific a language.

Easier if we provide injections for mkLuaInline, CC @horriblename

fwiw, I find flaw in both approaches. Ours require custom query rules for each option we want to support which is tedious, and theirs enforce a style of coding that is usually not in line with official language guidelines just to support a feature in a text editor plugin not everyone uses

Please don't do this

removed

NotAShelf · 2025-04-02T18:18:12Z

+              end
+            '';
+          description = ''
+            Table or function(lualinline) that will be passed to `conform.format()`. If this


Suggested change

Table or function(lualinline) that will be passed to `conform.format()`. If this

Table or function(lualinline) that will be passed to `conform.format()`. If this

Is this a function that takes luainline as an argument?

Is this a function that takes luainline as an argument?

it can take both

a table or a function that returns table

Okay, I'm mostly confused about the role of the paranthesis in the description. Does it indicate function arguments?

I think it's better to clarify the wording to state the types properly.

Suggested change

Table or function(lualinline) that will be passed to `conform.format()`. If this

Attribute set or or a Lua function that will be passed to `conform.format()`.

Or similar.

…appings.toggleFormatOnSave

venkyr77 · 2025-04-04T23:27:20Z

Diff addressing all the comments - https://github.com/NotAShelf/nvf/compare/7eec1d28d2a8f510964f304fb460884ef9f48b4c..0ffd8321e9fee37e3f191209921546a40523a269

venkyr77 · 2025-04-04T23:27:42Z

Diff addressing all the comments - https://github.com/NotAShelf/nvf/compare/7eec1d28d2a8f510964f304fb460884ef9f48b4c..0ffd8321e9fee37e3f191209921546a40523a269

@horriblename @NotAShelf

NotAShelf

Two small notes on the documentation. I have nothing further to nit (for once)

Perhaps a changelog entry would also go very well with the spirit of this change?

NotAShelf · 2025-04-04T23:38:16Z

+              end
+            '';
+          description = ''
+            Table or function(lualinline) that will be passed to `conform.format()`. If this


Okay, I'm mostly confused about the role of the paranthesis in the description. Does it indicate function arguments?

I think it's better to clarify the wording to state the types properly.

Suggested change

Table or function(lualinline) that will be passed to `conform.format()`. If this

Attribute set or or a Lua function that will be passed to `conform.format()`.

Or similar.

NotAShelf · 2025-04-04T23:38:47Z

+            Table or function(luainline) that will be passed to `conform.format()`. If this
+            is set, Conform will run the formatter asynchronously after save.


Same as my previous comment.

horriblename · 2025-04-05T02:23:28Z

+      in
+        mkOption {
+          type = nullOr (either attrs luaInline);
+          default = mkLuaInline ''


add defaultText please

something like

defaultText = literalMD "enabled, with fallback to LSP formatting that respects `vim.lsp.formatOnSave` and `vim.lsp.mappingd.toggleFormatSave`"

horriblename · 2025-05-13T21:07:17Z

superseded by #891

github-actions · 2025-05-13T21:07:37Z

✅ Preview has been deleted successfully!

venkyr77 requested review from NotAShelf, Soliprem and horriblename as code owners March 31, 2025 21:19

github-actions Bot pushed a commit that referenced this pull request Mar 31, 2025

Deploy PR #768 preview

847fba2

venkyr77 commented Mar 31, 2025

View reviewed changes

horriblename requested changes Mar 31, 2025

View reviewed changes

Comment thread modules/plugins/formatter/conform-nvim/conform-nvim.nix Outdated

Comment thread modules/plugins/lsp/config.nix Outdated

venkyr77 force-pushed the conform-format-on-save-fix branch 2 times, most recently from 943b343 to 7eec1d2 Compare April 2, 2025 08:18

github-actions Bot pushed a commit that referenced this pull request Apr 2, 2025

Deploy PR #768 preview

e8b7102

venkyr77 requested a review from horriblename April 2, 2025 17:18

horriblename reviewed Apr 2, 2025

View reviewed changes

NotAShelf requested changes Apr 2, 2025

View reviewed changes

Make conform respect config.vim.lsp.formatOnSave and config.vim.lsp.m…

0ffd832

…appings.toggleFormatOnSave

venkyr77 force-pushed the conform-format-on-save-fix branch from 7eec1d2 to 0ffd832 Compare April 4, 2025 23:21

github-actions Bot pushed a commit that referenced this pull request Apr 4, 2025

Deploy PR #768 preview

3477da0

venkyr77 requested review from NotAShelf and horriblename April 4, 2025 23:26

NotAShelf requested changes Apr 4, 2025

View reviewed changes

horriblename requested changes Apr 5, 2025

View reviewed changes

NotAShelf mentioned this pull request May 6, 2025

lsp: fix toggleFormatOnSave #869

Merged

17 tasks

horriblename mentioned this pull request May 9, 2025

conform: respect vim.lsp.formatOnSave and vim.lsp.mappings.toggleFormatOnSave #891

Merged

horriblename closed this May 13, 2025

github-actions Bot pushed a commit that referenced this pull request May 13, 2025

Deploy PR #768 preview

fde7cc4

	Table or function(lualinline) that will be passed to `conform.format()`. If this
	Table or function(lualinline) that will be passed to `conform.format()`. If this

	Table or function(lualinline) that will be passed to `conform.format()`. If this
	Attribute set or or a Lua function that will be passed to `conform.format()`.

		Table or function(luainline) that will be passed to `conform.format()`. If this
		is set, Conform will run the formatter asynchronously after save.

Uh oh!

Conversation

venkyr77 commented Mar 31, 2025

Testing:

Sanity Checking

Uh oh!

github-actions Bot commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Live preview deployed from 79993c7

Uh oh!

venkyr77 commented Mar 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

horriblename left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

venkyr77 commented Apr 4, 2025

Uh oh!

venkyr77 commented Apr 4, 2025

Uh oh!

NotAShelf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

horriblename commented May 13, 2025

Uh oh!

github-actions Bot commented May 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Mar 31, 2025 •

edited

Loading

🚀 Live preview deployed from `79993c7`