Help: Classes: code style update

[mtmccrea]

Note: This PR is meant for review of the implementation details, I've started a Discussion [#6307] to capture higher-level thoughts on whether this is a good idea, the scope, etc.

Purpose and Motivation

It stumbled into the project when playing with project-wide regex search and replace in VS Code, and before I knew it … 🕳️ 🐇

The general motivation is that the docs vary widely in quality and consistency of code examples, and this is a step toward cleaning it up.

There are almost no syntactic changes here, the vast majority of changes here are just handling whitespace. So in effect the examples just breathe a bit better, IMO. There are a few other advantages I see:

1. Carrying out the style guidelines makes the code generally more readable. Experienced users made lots of these examples, and their personal styles were imprinted in examples—it seems much of the “loose” formatting or compact style was convenient for them to type but not for users to read.
2. It sets an example for new users.
3. It makes it more likely that new contributions will follow the guidelines, and requests by reviewers to do so will hopefully feel less cumbersome.

Notes on scope

• Priority was given to rules that could be searched for. I.e. a rule may not be fully enacted if it wasn’t captured by a search—I didn’t go through files individually, with just a few exceptions.
• I’m limiting this to the Classes directory because
  1. that’s the highest-traffic set of doc pages, and
  2. I was halfway through “testing” this idea on that directory before I realized it would probably be feasible. By then, it had already been a good amount of work and I prioritized setting this reasonable and categorical scope rather than lose steam on the project having to replay it across all the docs.
     If successful, this PR can set a template for mapping onto other docs in the future (I’m roughly documenting the regex search-and-replace criteria)

Types of changes / implementation details

Each commit of the PR roughly corresponds to a rule or partial filling of the rule. See those for more details.

Here is a summary of the rules implemented:

• A large portion of the changes are summed up by this Rule:
  Add spaces within curly brackets {}, but not parentheses (), square brackets [], or argument lists ||.

• Use pipes instead of the arg keyword to express parameter lists.

• Space follows a comma (numeric chars on either side or if preceded by non-numeric char)

• Remove semicolon preceding a closing paren

• Remove space preceding comma

• Remove double spaces (tried to avoid this when there appeared to be an intentional alignment, e.g. aligning comments between lines)

• Add space after commas (exception: huge arrays)

• Surround equals signs with spaces

• Spaces follow semi colons when code is inline

• Spaces follow key operators

• Space follows comment slashes, and precedes them when inline

• No semicolon after the final statement of a function (recommendation)

• Removing double spaces before closing curly bracket

• Notably left out: spaces around operators - this was in part because, despite this being a rule, I could see this being a point of contention. And, admittedly, this is would be less straightforward to implement and likely a good deal more work. But, I’m happy to give it a go to catch a substantial number of instances of this.

• The arg keyword is replaced by pipes. It’s on my TODO list to restore the arg keyword in help files that may directly refer to it, but otherwise the pipes rule is enforced broadly. Some may have feelings about his.

To-do list

• Code is tested
• All tests are passing
• Updated documentation
• This PR is ready for review

Merge notes

• This should be squashed before merge. I'll do this myself once/if it's approved in order to preserve the history and regex criteria in the commit description

[mtmccrea]

A tip for reviewing the changes: take a broad look at the types of changes. Any individual change may seem trivial, but then in other instances it's (hopefully) more clear why this is useful.

This was a fairly automated task, with a human in the loop. There was plenty of bulk replace on a file-by-file basis, one rule or partial rule at a time. So the trivial and non-trivial changes came at equal cost.

[JordanHendersonMusic]

I've gone through about the top quarter or so files (alphabetically) in detail, and scanned through the rest, and I haven't been able to find any mistakes. I think it might be worth 'taking the plunge' and just merging this.

[capital-G]

Thanks for this!

I know that applying or enforcing styles is a controversial topic here, but I personally think that having a more consistent style throughout the documentation helps new users, and any improvement or work on the documentation is always welcomed.
The only downside I see is that the git history gets a bit more noisy, but I don't think that's a big problem for help files.

One thing I would like to add to such an effort would be a way for future commits to automatically apply this style as well - that is the beauty of a style guide, no more discussion and work to enforce a certain style.

You mentioned that you used regex to identify and modify the existing code - we could add this as a check/script via a git hook, so that any new commits would have to pass this check/script as well, which would also automatically suggest/apply any necessary changes by diffing them with your staged files. The CI could also tell us the style guide status of a commit in this case.
Basically: before you commit code, a script will verify if the changes concerning .schelp files pass the "test" and if not, the necessary changes will be applied to the file by the script - you can then review them (diff your staged files vs your current files) and add them to your commit by staging the automatic changes to your commit. I can make a small video of this in case people are interested and it is common approach in other projects.

I suggested something like this in #6096, but maybe it makes sense to try again here?

If others think this would be a good thing to implement, I could provide the necessary code, but in this case we should bundle the "git-hook" implementation with these changes.

[mtmccrea]

Thanks for having a look!

The only downside I see is that the git history gets a bit more noisy, but I don't think that's a big problem for help files.

I considered this, and it's really unavoidable. But the plan is to squash it to one commit on merge so that any given help file only has one blame commit in its history for this whole update. And I agree the doc files are low stakes relative to making this kind of change to the code base proper (otherwise I probably wouldn't have even undertaken the effort).

we could add this as a check/script via a git hook, so that any new commits would have to pass this check/script as well

I agree this would be a great addition! ... but out of scope for this PR and frankly beyond my capacity atm ;) It's a fruitful discussion to have though—it would be higher impact and worthwhile. Given the larger scope and changes to process that would imply I'll comment on that idea in the Discussion page.

[mtmccrea]

added: Space follows comment slashes, and precedes them when inline