Skip to content

Use KeyNamerWithOriginalFieldName function in jsonschema config generation #4418

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.

Sign up for GitHub

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

Draft
karimkhaleel wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Use KeyNamerWithOriginalFieldName function in jsonschema config generation #4418

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae7de0d
Use keyrenamer function in jsonschema config generation
karimkhaleel Mar 23, 2025
c233f07
fixup! Use keyrenamer function in jsonschema config generation
karimkhaleel Mar 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ require (
github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5
github.com/jesseduffield/minimal/gitignore v0.3.3-0.20211018110810-9cde264e6b1e
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0
github.com/karimkhaleel/jsonschema v0.0.0-20231001195015-d933f0d94ea3
github.com/karimkhaleel/jsonschema v0.0.0-20250323054317-7eb70f14797b
github.com/kyokomi/emoji/v2 v2.2.8
github.com/lucasb-eyer/go-colorful v1.2.0
github.com/mattn/go-runewidth v0.0.16
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1
github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk=
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 h1:iQTw/8FWTuc7uiaSepXwyf3o52HaUYcV+Tu66S3F5GA=
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0/go.mod h1:1NbS8ALrpOvjt0rHPNLyCIeMtbizbir8U//inJ+zuB8=
github.com/karimkhaleel/jsonschema v0.0.0-20231001195015-d933f0d94ea3 h1:s995u+gNQADMaixtNOs+jilRC/Q78q0UXSI7+4T0cDE=
github.com/karimkhaleel/jsonschema v0.0.0-20231001195015-d933f0d94ea3/go.mod h1:MCbEh21gjOzxc31udr3u4QM9DAdf8TFJCZz3u5hYIxA=
github.com/karimkhaleel/jsonschema v0.0.0-20250323054317-7eb70f14797b h1:fO3alvlIx8s0Ou+lkV6I46UzkUALHhdQYEWWqYMsirY=
github.com/karimkhaleel/jsonschema v0.0.0-20250323054317-7eb70f14797b/go.mod h1:MCbEh21gjOzxc31udr3u4QM9DAdf8TFJCZz3u5hYIxA=
github.com/kevinburke/ssh_config v0.0.0-20190725054713-01f96b0aa0cd h1:Coekwdh0v2wtGp9Gmz1Ze3eVRAWJMLokvN3QjdzCHLY=
github.com/kevinburke/ssh_config v0.0.0-20190725054713-01f96b0aa0cd/go.mod h1:CT57kijsi8u/K/BOFA39wgDQJ9CxiF4nAY/ojJ6r6mM=
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/user_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ type KeybindingCommitsConfig struct {
CherryPickCopy string `yaml:"cherryPickCopy"`
PasteCommits string `yaml:"pasteCommits"`
MarkCommitAsBaseForRebase string `yaml:"markCommitAsBaseForRebase"`
CreateTag string `yaml:"tagCommit"`
TagCommit string `yaml:"tagCommit"`
CheckoutCommit string `yaml:"checkoutCommit"`
ResetCherryPick string `yaml:"resetCherryPick"`
CopyCommitAttributeToClipboard string `yaml:"copyCommitAttributeToClipboard"`
Expand Down Expand Up @@ -963,7 +963,7 @@ func GetDefaultConfig() *UserConfig {
CherryPickCopy: "C",
PasteCommits: "V",
MarkCommitAsBaseForRebase: "B",
CreateTag: "T",
TagCommit: "T",
CheckoutCommit: "<space>",
ResetCherryPick: "<c-R>",
CopyCommitAttributeToClipboard: "y",
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/controllers/local_commits_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
Tooltip: self.c.Tr.RevertCommitTooltip,
},
{
Key: opts.GetKey(opts.Config.Commits.CreateTag),
Key: opts.GetKey(opts.Config.Commits.TagCommit),
Handler: self.withItem(self.createTag),
GetDisabledReason: self.require(self.singleItemSelected()),
Description: self.c.Tr.TagCommit,
Expand Down
2 changes: 1 addition & 1 deletion pkg/integration/tests/commit/create_tag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var CreateTag = NewIntegrationTest(NewIntegrationTestArgs{
Contains("two").IsSelected(),
Contains("one"),
).
Press(keys.Commits.CreateTag)
Press(keys.Commits.TagCommit)

t.ExpectPopup().CommitMessagePanel().
Title(Equals("Tag name")).
Expand Down
2 changes: 1 addition & 1 deletion pkg/integration/tests/tag/force_tag_annotated.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var ForceTagAnnotated = NewIntegrationTest(NewIntegrationTestArgs{
Contains("second commit").IsSelected(),
Contains("new-tag").Contains("first commit"),
).
Press(keys.Commits.CreateTag).
Press(keys.Commits.TagCommit).
Tap(func() {
t.ExpectPopup().CommitMessagePanel().
Title(Equals("Tag name")).
Expand Down
2 changes: 1 addition & 1 deletion pkg/integration/tests/tag/force_tag_lightweight.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var ForceTagLightweight = NewIntegrationTest(NewIntegrationTestArgs{
Contains("second commit").IsSelected(),
Contains("new-tag").Contains("first commit"),
).
Press(keys.Commits.CreateTag).
Press(keys.Commits.TagCommit).
Tap(func() {
t.ExpectPopup().CommitMessagePanel().
Title(Equals("Tag name")).
Expand Down
20 changes: 13 additions & 7 deletions pkg/jsonschema/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,14 @@ func getSubSchema(rootSchema, parentSchema *jsonschema.Schema, key string) *json
}

func customReflect(v *config.UserConfig) *jsonschema.Schema {
r := &jsonschema.Reflector{FieldNameTag: "yaml", RequiredFromJSONSchemaTags: true}
yamlToFieldNames := make(map[string]string)
keyNamer := func(yamlName string, originalFieldName string) string {
yamlToFieldNames[yamlName] = originalFieldName
yamlToFieldNames[originalFieldName] = yamlName

@stefanhaller stefanhaller Mar 23, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I find it extremely confusing that the same map is used for both directions. It works for us, but only because we never have something like this in our config:

type UserConfig struct {
	Foo string `yaml:"Bar"`
	Bar string `yaml:"Foo"`

That's a super contrived example, I know, but why not model this properly if we can.

I suggest a patch something like this:

diff --git i/pkg/jsonschema/generate.go w/pkg/jsonschema/generate.go
index ddb230207..54c711edb 100644
--- i/pkg/jsonschema/generate.go
+++ w/pkg/jsonschema/generate.go
@@ -53,9 +53,10 @@ func getSubSchema(rootSchema, parentSchema *jsonschema.Schema, key string) *json
 
 func customReflect(v *config.UserConfig) *jsonschema.Schema {
 	yamlToFieldNames := make(map[string]string)
+	fieldToYamlNames := make(map[string]string)
 	keyNamer := func(yamlName string, originalFieldName string) string {
 		yamlToFieldNames[yamlName] = originalFieldName
-		yamlToFieldNames[originalFieldName] = yamlName
+		fieldToYamlNames[originalFieldName] = yamlName
 		return yamlName
 	}
 
@@ -76,7 +77,7 @@ func customReflect(v *config.UserConfig) *jsonschema.Schema {
 
 		subSchema := getSubSchema(schema, userConfigSchema, yamlName)
 
-		setDefaultVals(schema, subSchema, defaultValue.FieldByName(fieldName).Interface(), yamlToFieldNames)
+		setDefaultVals(schema, subSchema, defaultValue.FieldByName(fieldName).Interface(), fieldToYamlNames)
 	}
 
 	return schema
@@ -92,7 +93,7 @@ func filterOutDevComments(r *jsonschema.Reflector) {
 	}
 }
 
-func setDefaultVals(rootSchema, schema *jsonschema.Schema, defaults any, yamlToFieldNames map[string]string) {
+func setDefaultVals(rootSchema, schema *jsonschema.Schema, defaults any, fieldToYamlNames map[string]string) {
 	t := reflect.TypeOf(defaults)
 	v := reflect.ValueOf(defaults)
 
@@ -123,7 +124,7 @@ func setDefaultVals(rootSchema, schema *jsonschema.Schema, defaults any, yamlToF
 		value := v.Field(i).Interface()
 		parentKey := t.Field(i).Name
 
-		key, ok := yamlToFieldNames[parentKey]
+		key, ok := fieldToYamlNames[parentKey]
 		if !ok {
 			fmt.Println(key)
 			continue
@@ -132,7 +133,7 @@ func setDefaultVals(rootSchema, schema *jsonschema.Schema, defaults any, yamlToF
 		subSchema := getSubSchema(rootSchema, schema, key)
 
 		if isStruct(value) {
-			setDefaultVals(rootSchema, subSchema, value, yamlToFieldNames)
+			setDefaultVals(rootSchema, subSchema, value, fieldToYamlNames)
 		} else if !isZeroValue(value) {
 			subSchema.Default = value
 		}

@karimkhaleel karimkhaleel Mar 23, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I pushed a fixup where I used lo.Invert for this.

@stefanhaller stefanhaller Mar 23, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Even better, nice.

@karimkhaleel karimkhaleel Mar 23, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can still run into a bit of a naming issue here if we have something like this:

type ConfigA struct {
    Color string `yaml:"color"`
}
type ConfigB struct {
    Color string `yaml:"shade"`
}

or

type ConfigA struct {
    Shade string `yaml:"color"`
}
type ConfigB struct {
    Color string `yaml:"color"`
}

We could get away with something like this in the OriginalPropertiesMapping approach since we maintained the relationship hierarchy, but since we are storing a flat map here, this relationship is lost.

return yamlName
}

r := &jsonschema.Reflector{FieldNameTag: "yaml", RequiredFromJSONSchemaTags: true, KeyNamerWithOriginalFieldName: keyNamer}
if err := r.AddGoComments("github.com/jesseduffield/lazygit/pkg/config", "../config"); err != nil {
panic(err)
}
Expand All @@ -63,15 +70,13 @@ func customReflect(v *config.UserConfig) *jsonschema.Schema {

defaultValue := reflect.ValueOf(defaultConfig).Elem()

yamlToFieldNames := lo.Invert(userConfigSchema.OriginalPropertiesMapping)

for pair := userConfigSchema.Properties.Oldest(); pair != nil; pair = pair.Next() {
yamlName := pair.Key
fieldName := yamlToFieldNames[yamlName]

subSchema := getSubSchema(schema, userConfigSchema, yamlName)

setDefaultVals(schema, subSchema, defaultValue.FieldByName(fieldName).Interface())
setDefaultVals(schema, subSchema, defaultValue.FieldByName(fieldName).Interface(), yamlToFieldNames)
}

return schema
Expand All @@ -87,7 +92,7 @@ func filterOutDevComments(r *jsonschema.Reflector) {
}
}

func setDefaultVals(rootSchema, schema *jsonschema.Schema, defaults any) {
func setDefaultVals(rootSchema, schema *jsonschema.Schema, defaults any, yamlToFieldNames map[string]string) {
t := reflect.TypeOf(defaults)
v := reflect.ValueOf(defaults)

Expand Down Expand Up @@ -118,15 +123,16 @@ func setDefaultVals(rootSchema, schema *jsonschema.Schema, defaults any) {
value := v.Field(i).Interface()
parentKey := t.Field(i).Name

key, ok := schema.OriginalPropertiesMapping[parentKey]
key, ok := yamlToFieldNames[parentKey]
if !ok {
fmt.Println(key)
continue
}

subSchema := getSubSchema(rootSchema, schema, key)

if isStruct(value) {
setDefaultVals(rootSchema, subSchema, value)
setDefaultVals(rootSchema, subSchema, value, yamlToFieldNames)
} else if !isZeroValue(value) {
subSchema.Default = value
}
Expand Down
21 changes: 10 additions & 11 deletions vendor/github.com/karimkhaleel/jsonschema/.golangci.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 11 additions & 5 deletions vendor/github.com/karimkhaleel/jsonschema/README.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

93 changes: 0 additions & 93 deletions vendor/github.com/karimkhaleel/jsonschema/comment_extractor.go
View file
Open in desktop

This file was deleted.

Loading