Skip to content

Add diff output to meson format --check-only #14504

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

julianneswinoga
Copy link

Currently meson format --check-only exits without any output if it fails. This implements some simple diff output so the user knows why it failed 🙂

$ meson.py format --check-only
--- meson.build (original)
+++ meson.build (reformatted)
@@ -1,5 +1,4 @@
-project('my_project', 'c'
-  )
+project('my_project', 'c')

 a = declare_dependency(dependencies: dependency('threads'))
 b = declare_dependency(dependencies: a)

Pretty simple change but I think it makes a huge DevEx improvement.

@dcbaker dcbaker added this to the 1.9 milestone Apr 21, 2025
@dcbaker
Copy link
Member

dcbaker commented Apr 21, 2025

This all looks good and correct.

I think the todo being fixed here asking for a verbose mode (or a quiet mode, I don't really care which) is a good one. So adding either a --verbose that shows the diff, or a --quiet the suppresses it would be good.

@julianneswinoga
Copy link
Author

Good point, unfortunately -q is the short form of --check-only (for some reason) so I opted for -s/--silent

Copy link
Member

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

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

Thanks!

This looks good to me, it'll have to wait until we leave feature freeze for 1.8, but I've added it to the 1.9 milestone.

@julianneswinoga julianneswinoga force-pushed the feature/jswinoga/format-difflib-output branch from 9a536b4 to cc7186d Compare April 21, 2025 20:51
Copy link
Member

@eli-schwartz eli-schwartz left a comment

Choose a reason for hiding this comment

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

Overall I really like the idea of this feature. That being said...

@julianneswinoga julianneswinoga force-pushed the feature/jswinoga/format-difflib-output branch from cc7186d to e60faac Compare April 21, 2025 22:27
@bruchar1
Copy link
Member

Good point, unfortunately -q is the short form of --check-only (for some reason) so I opted for -s/--silent

This was to use the same syntax as muon format. I think the logic was that the --check-only mode is quiet as is does not output the formatted file.

Would it be better to add a '-d', '--check-diff' option, and to keep the -q option silent by default? I think I would prefer that.

@julianneswinoga julianneswinoga force-pushed the feature/jswinoga/format-difflib-output branch from e60faac to ad22acf Compare April 25, 2025 15:24
@julianneswinoga
Copy link
Author

Sure, makes sense. Updated 🙂

@julianneswinoga julianneswinoga force-pushed the feature/jswinoga/format-difflib-output branch from ad22acf to 7a80dd4 Compare April 25, 2025 18:02
Copy link
Member

@bruchar1 bruchar1 left a comment

Choose a reason for hiding this comment

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

LGTM. You should add a release snippet (just create a md file in docs/markdown/snippets) and update Commands.md page in the manual.

@julianneswinoga julianneswinoga force-pushed the feature/jswinoga/format-difflib-output branch from 7a80dd4 to b7e4d10 Compare April 25, 2025 18:55
Copy link
Member

@bruchar1 bruchar1 left a comment

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants