Contrib.social

Decide between autosave and auto-save and standardize

Open on Drupal.org →
Created on 12 March 2025, 13 days ago

tl;dr

We should start using only the hyphenated "auto-save" and its variants--auto_save, and AutoSave--and make any adjustments to existing code that seem practical and beneficial.

Overview

We're currently inconsistent in our spelling of autosave/auto-save throughout the codebase. Most of the time it's just a minor annoyance, but it can make text search harder and even result in inconsistent APIs and endpoints. It violates one of the best-named software rules of all time: The Principle of Least Surprise. 😛 Here's a quick breakdown of the situation:

Routes

Four of each:

Filenames

Non-test files:

$ for i in autosave auto-save auto_save AutoSave; do echo "$i: $(find . -name \*"$i"\* -not -path \*test\* | wc -l)"; done

autosave:   0
auto-save:  0
auto_save:  1
AutoSave:   7

Specifically...

$ for i in autosave auto-save auto_save AutoSave; do echo "$i:"; find . -name \*"$i"\* -not -path \*test\*; echo; done

autosave:

auto-save:

auto_save:
./experience_builder.auto_save.inc

AutoSave:
./src/AutoSaveData.php
./src/Controller/ApiAutoSaveController.php
./src/Controller/ApiConfigAutoSaveControllers.php
./src/AutoSave
./src/AutoSave/AutoSaveManager.php
./src/AutoSave/AutoSaveTempStore.php
./src/AutoSave/AutoSaveTempStoreFactory.php

Total code occurrences

Including code comments and variable names--which are kind of all over the place--in non-test files:

$ for i in autosave auto-save auto_save AutoSave; do echo "$i: $(find . -name '*'"$i"'*' -not -name Test.php | wc -l)"; done    

autosave:    1
auto-save:   0
auto_save:   1
AutoSave:   13

Other

  • At least a few JsonResponse-es have autosave in a key.
  • At least one JsonResponse has "auto-save" in a message string.
  • The "Component" field for Drupal.org issues is "Auto-save".

Proposed resolution

My conclusion: The precedent strongly favors two words, either separated with a hypen or underscore or camel-cased. Unfortunately, there are four API-impacting instances--four routes and a few API data keys that include the one-word form. Unless-and-until we start versioning the API, we'll probably just have to live with those. Other than that, I recommend...

  1. Standardize on the hyphenated "auto-save" and its variants--auto_save, and AutoSave.
  2. Document the decision.
  3. Decide if we need to make any changes to existing code/documentation.
  4. Determine if there are any UI strings that we want to change.
  5. Let the dev team know about it.

User interface changes

TBD

📌 Task
Status

Active

Version

0.0

Component

Page builder

Created by

🇺🇸United States traviscarden

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @traviscarden
  • Comment 13 days ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    One more thing: the issue queue component is also hyphenated, because that seemed to match the dominant spelling.

    We can change the APIs (PHP API, internal HTTP API) easily at this alpha stage.

    Let's make it consistent, and execute your conclusion 🙏 So that means:

    1. 👍 Standardize on the hyphenated "auto-save" and its variants--auto_save, and AutoSave.
    2. 👍 Document the decision.
    3. Yes, let's make this consistent too.
    4. 👍 Determine if there are any UI strings that we want to change.
    5. No need, we can just do it.

    Ideally, we'd be able to configure cspell to forbid autosave and Autosave, but allow auto-save, Auto-save and Auto-Save. That'd catch everything AFAICT?

  • Merge request !779Standardize on hyphenated spelling of "auto-save" → (Merged) created by traviscarden
  • Comment 12 days ago →
    🇺🇸United States traviscarden

    I think I got all but one thing, @wim leers: Where should we document the standard? I didn't see a place that it seemed like it fit.

    I've updated the issue summary to clarify that I mean to let the dev team know about it after it's been committed, just to prevent surprise and confusion.

    Also, I want credit for the fact that my initial analysis in the issue summary already included the issue queue component. I don't want that counting against me if you and I ever get into some kind of contest, @wim leers. 😉

    Finally, I see the cpsell job failed. Am I crazy, or did it fail because it found a forbidden word in the directive with which I forbad it in dictionary.txt? 🤨 Well, maybe it's an easy fix--maybe the inline comments confuse it... but it's time for me to log off for the night. @wim leers, maybe you know the solution off the top of your head.

  • Comment 12 days ago →
    🇪🇸Spain fjgarlin

    You might need to actually ignore the project's dictionary.txt from the cspell configuration.

  • Comment 12 days ago →
    🇪🇸Spain fjgarlin

    Cspell job fixed after the last commit.

  • Pipeline finished with Failed
    12 days ago
    Total: 1610s
    #447263
  • Comment 11 days ago →
    🇺🇸United States traviscarden

    I guess that makes sense, @fjgarlin, since these are the first negative terms--in other words, dictionary.txt was probably already being scanned, but it always passed because it contained all of its own terms. 😛

    So the tests pass now. Bumping priority since this affects APIs, and we don't want to start to drift and get out of sync. On that account, I'm going to go ahead and assign directly to @wim leers to see if we can rush it through. Ready for review!

  • Comment 11 days ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #4: I suggested the dictionary as the way to both enforce and document it.

    Thanks for getting this done so quickly!

  • Pipeline finished with Skipped
    11 days ago
    #447756
  • Comment 11 days ago →
    System Message
  • Comment 11 days ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Thanks, fellow nitpicker! 🤓😄

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024