[LinqLover]

With this patch, users can create a prefs/gitSUser file relative to their image and store a default author name, e-mail address, and any number of credentials for URL patterns in it. git-s automatically checks this file before asking the user for this information. This is especially helpful for power users who often use a fresh Squeak image to implement new things and then every time create new git repositories in this image.

[Copilot]

🟡 Changes recommended

Credential parsing and retry recovery have correctness defects, and author lookups break standalone FileSystemGit use.

4 open issues

• Missing optional settings provider breaks email requests · New
• Unconditional settings lookup breaks standalone FileSystemGit · New
• Credential parsing truncates passwords at colons · New
• Retry reuses rejected external credentials · New

What changed in this PR

This PR adds optional prefs/gitSUser settings so git-s can reuse author information and URL-matched credentials across fresh Squeak images.

Changes:

• Adds an external-settings provider with credential parsing and lifecycle support.
• Uses external settings for author defaults and credential selection.
• Loads settings immediately after installation through a migration.

File	Description
src/​GitS-Core.package/​GSGitWorkingCopy.class/​methodProperties.json	Updates credential-method metadata.
src/​GitS-Core.package/​GSGitWorkingCopy.class/​instance/​getCredentialsFor..st	Automatically selects URL-matched external credentials.
src/​GitS-Core.package/​GSGitWorkingCopy.class/​class/​formatReusableCredentials.usedIn.detailed..st	Labels credentials from external settings.
src/​GitS-Core.package/​GSGitWorkingCopy.class/​class/​findCredentialsFor.ifNone..st	Includes external credentials among reuse options.
src/​GitS-Core.package/​GSExternalSettings.class/​README.md	Documents the settings provider.
src/​GitS-Core.package/​GSExternalSettings.class/​properties.json	Defines the settings-provider class.
src/​GitS-Core.package/​GSExternalSettings.class/​methodProperties.json	Registers new method metadata.
src/​GitS-Core.package/​GSExternalSettings.class/​class/​settingsKeys.st	Exposes loaded settings keys.
src/​GitS-Core.package/​GSExternalSettings.class/​class/​settingsFileName.st	Defines the gitSUser filename.
src/​GitS-Core.package/​GSExternalSettings.class/​class/​releaseExternalSettings.st	Clears loaded settings.
src/​GitS-Core.package/​GSExternalSettings.class/​class/​initialize.st	Registers with ExternalSettings.
src/​GitS-Core.package/​GSExternalSettings.class/​class/​fetchExternalSettingsIn..st	Reads the optional settings file.
src/​GitS-Core.package/​GSExternalSettings.class/​class/​credentials.st	Documents and parses account entries.
src/​GitS-Core.package/​GSExternalSettings.class/​class/​at.ifPresent.ifAbsent..st	Provides conditional settings lookup.
src/​GitS-Core.package/​GSExternalSettings.class/​class/​at.ifPresent..st	Adds a presence-only lookup helper.
src/​GitS-Core.package/​GSExternalSettings.class/​class/​at..st	Provides direct settings lookup.
src/​FileSystem-Git.package/​GitStamp.class/​methodProperties.json	Updates author-accessor metadata.
src/​FileSystem-Git.package/​GitStamp.class/​class/​defaultName.st	Consults external author-name settings.
src/​FileSystem-Git.package/​GitStamp.class/​class/​defaultEmail.st	Consults external author-email settings.
src/​BaselineOfGitS.package/​BaselineOfGitS.class/​methodProperties.json	Registers migration metadata.
src/​BaselineOfGitS.package/​BaselineOfGitS.class/​instance/​initializeExternalSettings.st	Loads settings without requiring an image restart.

_{Review effort: Balanced}

[LinqLover]

That looks like an issue, but not a regression from this change?

[MariusDoe]

I actually think the AI was right on this one: GSGitWorkingCopy>>handleCredentialsDuring: removes invalid credentials, but that only works for the in-image store, not for GSExternalSettings. But since be24d1f I think this should not be a problem anymore. Feel free to resolve the conversation if you agree :)

[MariusDoe]

Actually, I spoke too soon. I think this problem still applies, for the same reasoning as above. The commit I mentioned changed storing externally sourced credentials in the in-image cache to not storing them, but the second round of handling credentials will still re-invoke the external settings logic and thus retry the same credentials again.

I think one solution to this problem may be to store a sentinel (e.g. #invalid) instead of removing the association in removeCredentialsFor: (then the method should probably be renamed to something like invalidateCredentialsFor:). That way, the next round will find that instead of trying the external settings. Although we will need some logic to allow the user to replace that sentinel later on. Maybe when getCredentialsFor: finds the sentinel, it asks the user to input a new one or suggests removing the sentinel ("I have updated the credentials in the external preferences"). The immediate path (where removeCredentialsFor: is called) should probably also be changed to include that new suggestion.

What do you think?

[LinqLover]

#########################
# 1 tests did not pass: #
#########################

GSLintTest
 ✗ #testNoUnimplementedAndNoUnsentMethods (4226ms)
TestFailure: Found unimplemented selectors or references to non-existing globals: an OrderedCollection((GSGitWorkingCopy class>>#findCredentialsFor:ifNone: "a CompiledMethod(2488921)")->#externalSettings)
GSLintTest(TestCase)>>assert:description:
GSLintTest>>testNoUnimplementedAndNoUnsentMethods ...assert: unimplemented isEmpty
		description: ('Found unimplemented selectors or references to non-existing globals: {1}' format: {unimplemented})
GSLintTest(TestCase)>>performTest

Maybe GSSendsEnumerator should not every literal as a sent when it is actually just used as a value? If that's too expensive, add a expectedUnimplementedMethods instead?

[MariusDoe]

Thanks for the PR! Interesting, I did not know about ExternalSettings. If you think this is useful, we can certainly merge it. One general question: what is the intended way to populate the external settings? Should the getCredentialsFor: path somehow offer to store the credentials in the external file?

Maybe GSSendsEnumerator should not every literal as a sent when it is actually just used as a value? If that's too expensive, add a expectedUnimplementedMethods instead?

There is actually already logic to handle "literals as values". Since it's not that easy to detect which symbol literals are used as values and which as shorthand BlockClosures, I have added <linterIgnoreSends: #(...)> (example use) and selectorsWhoseReceiverAndArgumentsToIgnore. I think I'd go for the former in this case, instead of adding at:ifAbsentPut: to the latter :)

[LinqLover]

Thanks for the review!

One general question: what is the intended way to populate the external settings? Should the getCredentialsFor: path somehow offer to store the credentials in the external file?

I'm not aware of a way for this. :-) I always edited the settings file manually. I believe this would require further thinking about the general ExternalSettings framework, and something we could address later (first in the trunk, then complement in git/s).

Does my postload migration look correct? I was not quite sure. :-)

Will look at your remaining comments soon!