Skip to content

Conversation

@Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Dec 4, 2025

Tackled a long-standing pet-peeve by trimming all files of legacy vim/emac declarations. This is an antiquated manner of handling style data, as all modern tools parse .editorconfig instead. This change is excluded in .git-blame-ignore-revs to prevent any Git noise.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

@mwichmann
Copy link
Collaborator

Ambitious.

Yeah, obviously that stanza long predates the existence of the editorconfig spec - and shows the reason for it! I'm neutral on whether this is actually "worth doing", I was originally going to dive right in, then backed off. Up to the boss, then :-)

@mwichmann mwichmann added the maintenance Tasks to maintain internal SCons code/tools label Dec 4, 2025
@mwichmann
Copy link
Collaborator

Looks like the (rather fragile) Interactive tests got broken here. This has happened before. These files can't auto-strip trailing blanks because there are expect blocks that have necessary trailing space, like

expect_stdout = """\↵
scons>>> Copy("foo.out", "foo.in")↵
scons>>> Touch("1")↵
scons>>> Copy("foo.out", "foo.in")↵
scons>>> Touch("2")↵
scons>>> scons: `foo-alias' is up to date.↵
scons>>> ↵
"""

Note the trailing space on the last of those.

@mwichmann
Copy link
Collaborator

I imagine we could edit those fairly easily to avoid... say: scons>>> \n"""

Not tested.

@bdbaddog
Copy link
Contributor

bdbaddog commented Dec 5, 2025

Does current vim/emacs read the .editorconfig file?

@mwichmann
Copy link
Collaborator

Does current vim/emacs read the .editorconfig file?

https://editorconfig.org/#pre-installed

tl;dr: yes.

(dunno why they put that section in alphabetical order except emacs which is at the end)

@kprussing
Copy link
Contributor

Looks like vim 9 ships the plugin, but 8.0 (default RHEL 8) does not ship the plugin.

@Repiteo Repiteo force-pushed the vim-emac-purge branch 2 times, most recently from adb7bda to f65c46a Compare December 5, 2025 15:52
@Repiteo
Copy link
Contributor Author

Repiteo commented Dec 5, 2025

I imagine we could edit those fairly easily to avoid... say: scons>>> \n"""

Not tested.

Looks like that would mostly fix the issue, but a few of the files still failed:

Failed the following 3 tests:
	test/Interactive/cache-debug.py
	test/Interactive/taskmastertrace.py
	test/Interactive/version.py

That tweak was already pushing it, but anything beyond that would absolutely be outside the scope of this PR, so I opted to just re-add the extra newlines

@mwichmann
Copy link
Collaborator

Looks like that would mostly fix the issue, but a few of the files still failed:

Yes, I see the second and third of those have an empty scons prompt line (thus ending with space) in a place that isn't the last line of the expect block, so indeed that hack wouldn't finish fixing up those. There are other ways - we'll think about it. As I said, that's happened before - I've had space-stripping for Python enabled for years, before we had an SCons-specific editorconfig.

@mwichmann
Copy link
Collaborator

Okay, this is odd... those files shouldn't have gotten broken due to editing them as the .editorconfig in the project contains:

[test/Interactive/**]                                                        
trim_trailing_whitespace = false  

Is there some reason that didn't work to protect them? Bad syntax or something?

@Repiteo
Copy link
Contributor Author

Repiteo commented Dec 5, 2025

That's a VSCode issue iirc, where .editorconfig is overridden when it comes to the global trailing whitespace option specifically. It's what I used to do the regex search & replace, and despite disabling ruff/mypy I failed to consider the general formatting options

@mwichmann
Copy link
Collaborator

mwichmann commented Dec 6, 2025

Looks like vim 9 ships the plugin, but 8.0 (default RHEL 8) does not ship the plugin.

@kprussing you're right. Not yet sure how to process that bit of info... RHEL 8 is past the end of one if its lives, but still gets maintenance support (security fixes) through 2029.

Guess I'm not sure someone pegged to RHEL/Alma/Rocky 8 is going to be actively editing SCons itself...

@kprussing
Copy link
Contributor

Guess I'm not sure someone pegged to RHEL/Alma/Rocky 8 is going to be actively editing SCons itself...

Probably true. If they're doing more than just hacking a local copy, I would suspect they would try to migrate the changes back and follow the rules.

Using the .editorconfig also assumes the extensions/plugins are actually enabled by the author.

@mwichmann
Copy link
Collaborator

Using the .editorconfig also assumes the extensions/plugins are actually enabled by the author.

I think we can put some kind of code-style suggestion in the CONTRIbUTING file - like 'enable editorconfig support if available, otherwise these settings".

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

Labels

maintenance Tasks to maintain internal SCons code/tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants