diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index e7d884dbb..12b377f96 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -5,6 +5,7 @@ import ( "log" "os" "path/filepath" + "reflect" "strings" "time" @@ -237,7 +238,17 @@ func migrateUserConfig(path string, content []byte) ([]byte, error) { // A pure function helper for testing purposes func computeMigratedConfig(path string, content []byte) ([]byte, error) { - changedContent := content + var err error + var rootNode yaml.Node + err = yaml.Unmarshal(content, &rootNode) + if err != nil { + return nil, fmt.Errorf("failed to parse YAML: %w", err) + } + var originalCopy yaml.Node + err = yaml.Unmarshal(content, &originalCopy) + if err != nil { + return nil, fmt.Errorf("failed to parse YAML, but only the second time!?!? How did that happen: %w", err) + } pathsToReplace := []struct { oldPath []string @@ -248,46 +259,52 @@ func computeMigratedConfig(path string, content []byte) ([]byte, error) { {[]string{"gui", "windowSize"}, "screenMode"}, } - var err error for _, pathToReplace := range pathsToReplace { - changedContent, err = yaml_utils.RenameYamlKey(changedContent, pathToReplace.oldPath, pathToReplace.newName) + err := yaml_utils.RenameYamlKey(&rootNode, pathToReplace.oldPath, pathToReplace.newName) if err != nil { return nil, fmt.Errorf("Couldn't migrate config file at `%s` for key %s: %s", path, strings.Join(pathToReplace.oldPath, "."), err) } } - changedContent, err = changeNullKeybindingsToDisabled(changedContent) + err = changeNullKeybindingsToDisabled(&rootNode) if err != nil { return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err) } - changedContent, err = changeElementToSequence(changedContent, []string{"git", "commitPrefix"}) + err = changeElementToSequence(&rootNode, []string{"git", "commitPrefix"}) if err != nil { return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err) } - changedContent, err = changeCommitPrefixesMap(changedContent) + err = changeCommitPrefixesMap(&rootNode) if err != nil { return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err) } + // Add more migrations here... - return changedContent, nil + if !reflect.DeepEqual(rootNode, originalCopy) { + newContent, err := yaml_utils.YamlMarshal(&rootNode) + if err != nil { + return nil, fmt.Errorf("Failed to remarsal!\n %w", err) + } + return newContent, nil + } else { + return content, nil + } } -func changeNullKeybindingsToDisabled(changedContent []byte) ([]byte, error) { - return yaml_utils.Walk(changedContent, func(node *yaml.Node, path string) bool { +func changeNullKeybindingsToDisabled(rootNode *yaml.Node) error { + return yaml_utils.Walk(rootNode, func(node *yaml.Node, path string) { if strings.HasPrefix(path, "keybinding.") && node.Kind == yaml.ScalarNode && node.Tag == "!!null" { node.Value = "" node.Tag = "!!str" - return true } - return false }) } -func changeElementToSequence(changedContent []byte, path []string) ([]byte, error) { - return yaml_utils.TransformNode(changedContent, path, func(node *yaml.Node) (bool, error) { +func changeElementToSequence(rootNode *yaml.Node, path []string) error { + return yaml_utils.TransformNode(rootNode, path, func(node *yaml.Node) error { if node.Kind == yaml.MappingNode { nodeContentCopy := node.Content node.Kind = yaml.SequenceNode @@ -298,15 +315,14 @@ func changeElementToSequence(changedContent []byte, path []string) ([]byte, erro Content: nodeContentCopy, }} - return true, nil + return nil } - return false, nil + return nil }) } -func changeCommitPrefixesMap(changedContent []byte) ([]byte, error) { - return yaml_utils.TransformNode(changedContent, []string{"git", "commitPrefixes"}, func(prefixesNode *yaml.Node) (bool, error) { - changedAnyNodes := false +func changeCommitPrefixesMap(rootNode *yaml.Node) error { + return yaml_utils.TransformNode(rootNode, []string{"git", "commitPrefixes"}, func(prefixesNode *yaml.Node) error { if prefixesNode.Kind == yaml.MappingNode { for _, contentNode := range prefixesNode.Content { if contentNode.Kind == yaml.MappingNode { @@ -318,11 +334,10 @@ func changeCommitPrefixesMap(changedContent []byte) ([]byte, error) { Kind: yaml.MappingNode, Content: nodeContentCopy, }} - changedAnyNodes = true } } } - return changedAnyNodes, nil + return nil }) } diff --git a/pkg/config/app_config_test.go b/pkg/config/app_config_test.go index cf72f201b..9f9abc38a 100644 --- a/pkg/config/app_config_test.go +++ b/pkg/config/app_config_test.go @@ -87,3 +87,611 @@ git: }) } } + +var largeConfiguration = []byte(` +# Config relating to the Lazygit UI +gui: + # The number of lines you scroll by when scrolling the main window + scrollHeight: 2 + + # If true, allow scrolling past the bottom of the content in the main window + scrollPastBottom: true + + # See https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#scroll-off-margin + scrollOffMargin: 2 + + # One of: 'margin' (default) | 'jump' + scrollOffBehavior: margin + + # The number of spaces per tab; used for everything that's shown in the main view, but probably mostly relevant for diffs. + # Note that when using a pager, the pager has its own tab width setting, so you need to pass it separately in the pager command. + tabWidth: 4 + + # If true, capture mouse events. + # When mouse events are captured, it's a little harder to select text: e.g. requiring you to hold the option key when on macOS. + mouseEvents: true + + # If true, do not show a warning when discarding changes in the staging view. + skipDiscardChangeWarning: false + + # If true, do not show warning when applying/popping the stash + skipStashWarning: false + + # If true, do not show a warning when attempting to commit without any staged files; instead stage all unstaged files. + skipNoStagedFilesWarning: false + + # If true, do not show a warning when rewording a commit via an external editor + skipRewordInEditorWarning: false + + # Fraction of the total screen width to use for the left side section. You may want to pick a small number (e.g. 0.2) if you're using a narrow screen, so that you can see more of the main section. + # Number from 0 to 1.0. + sidePanelWidth: 0.3333 + + # If true, increase the height of the focused side window; creating an accordion effect. + expandFocusedSidePanel: false + + # The weight of the expanded side panel, relative to the other panels. 2 means + # twice as tall as the other panels. Only relevant if expandFocusedSidePanel is true. + expandedSidePanelWeight: 2 + + # Sometimes the main window is split in two (e.g. when the selected file has both staged and unstaged changes). This setting controls how the two sections are split. + # Options are: + # - 'horizontal': split the window horizontally + # - 'vertical': split the window vertically + # - 'flexible': (default) split the window horizontally if the window is wide enough, otherwise split vertically + mainPanelSplitMode: flexible + + # How the window is split when in half screen mode (i.e. after hitting '+' once). + # Possible values: + # - 'left': split the window horizontally (side panel on the left, main view on the right) + # - 'top': split the window vertically (side panel on top, main view below) + enlargedSideViewLocation: left + + # If true, wrap lines in the staging view to the width of the view. This + # makes it much easier to work with diffs that have long lines, e.g. + # paragraphs of markdown text. + wrapLinesInStagingView: true + + # One of 'auto' (default) | 'en' | 'zh-CN' | 'zh-TW' | 'pl' | 'nl' | 'ja' | 'ko' | 'ru' + language: auto + + # Format used when displaying time e.g. commit time. + # Uses Go's time format syntax: https://pkg.go.dev/time#Time.Format + timeFormat: 02 Jan 06 + + # Format used when displaying time if the time is less than 24 hours ago. + # Uses Go's time format syntax: https://pkg.go.dev/time#Time.Format + shortTimeFormat: 3:04PM + + # Config relating to colors and styles. + # See https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#color-attributes + theme: + # Border color of focused window + activeBorderColor: + - green + - bold + + # Border color of non-focused windows + inactiveBorderColor: + - default + + # Border color of focused window when searching in that window + searchingActiveBorderColor: + - cyan + - bold + + # Color of keybindings help text in the bottom line + optionsTextColor: + - blue + + # Background color of selected line. + # See https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#highlighting-the-selected-line + selectedLineBgColor: + - blue + + # Background color of selected line when view doesn't have focus. + inactiveViewSelectedLineBgColor: + - bold + + # Foreground color of copied commit + cherryPickedCommitFgColor: + - blue + + # Background color of copied commit + cherryPickedCommitBgColor: + - cyan + + # Foreground color of marked base commit (for rebase) + markedBaseCommitFgColor: + - blue + + # Background color of marked base commit (for rebase) + markedBaseCommitBgColor: + - yellow + + # Color for file with unstaged changes + unstagedChangesColor: + - red + + # Default text color + defaultFgColor: + - default + + # Config relating to the commit length indicator + commitLength: + # If true, show an indicator of commit message length + show: true + + # If true, show the '5 of 20' footer at the bottom of list views + showListFooter: true + + # If true, display the files in the file views as a tree. If false, display the files as a flat list. + # This can be toggled from within Lazygit with the '' key, but that will not change the default. + showFileTree: true + + # If true, show the number of lines changed per file in the Files view + showNumstatInFilesView: false + + # If true, show a random tip in the command log when Lazygit starts + showRandomTip: true + + # If true, show the command log + showCommandLog: true + + # If true, show the bottom line that contains keybinding info and useful buttons. If false, this line will be hidden except to display a loader for an in-progress action. + showBottomLine: true + + # If true, show jump-to-window keybindings in window titles. + showPanelJumps: true + + # Deprecated: use nerdFontsVersion instead + showIcons: false + + # Nerd fonts version to use. + # One of: '2' | '3' | empty string (default) + # If empty, do not show icons. + nerdFontsVersion: "" + + # If true (default), file icons are shown in the file views. Only relevant if NerdFontsVersion is not empty. + showFileIcons: true + + # Length of author name in (non-expanded) commits view. 2 means show initials only. + commitAuthorShortLength: 2 + + # Length of author name in expanded commits view. 2 means show initials only. + commitAuthorLongLength: 17 + + # Length of commit hash in commits view. 0 shows '*' if NF icons aren't on. + commitHashLength: 8 + + # If true, show commit hashes alongside branch names in the branches view. + showBranchCommitHash: false + + # Whether to show the divergence from the base branch in the branches view. + # One of: 'none' | 'onlyArrow' | 'arrowAndNumber' + showDivergenceFromBaseBranch: none + + # Height of the command log view + commandLogSize: 8 + + # Whether to split the main window when viewing file changes. + # One of: 'auto' | 'always' + # If 'auto', only split the main window when a file has both staged and unstaged changes + splitDiff: auto + + # Default size for focused window. Can be changed from within Lazygit with '+' and '_' (but this won't change the default). + # One of: 'normal' (default) | 'half' | 'full' + screenMode: normal + + # Window border style. + # One of 'rounded' (default) | 'single' | 'double' | 'hidden' + border: rounded + + # If true, show a seriously epic explosion animation when nuking the working tree. + animateExplosion: true + + # Whether to stack UI components on top of each other. + # One of 'auto' (default) | 'always' | 'never' + portraitMode: auto + + # How things are filtered when typing '/'. + # One of 'substring' (default) | 'fuzzy' + filterMode: substring + + # Config relating to the spinner. + spinner: + # The frames of the spinner animation. + frames: + - '|' + - / + - '-' + - \ + + # The "speed" of the spinner in milliseconds. + rate: 50 + + # Status panel view. + # One of 'dashboard' (default) | 'allBranchesLog' + statusPanelView: dashboard + + # If true, jump to the Files panel after popping a stash + switchToFilesAfterStashPop: true + + # If true, jump to the Files panel after applying a stash + switchToFilesAfterStashApply: true + + # If true, when using the panel jump keys (default 1 through 5) and target panel is already active, go to next tab instead + switchTabsWithPanelJumpKeys: false + +# Config relating to git +git: + # See https://github.com/jesseduffield/lazygit/blob/master/docs/Custom_Pagers.md + paging: + # Value of the --color arg in the git diff command. Some pagers want this to be set to 'always' and some want it set to 'never' + colorArg: always + + # e.g. + # diff-so-fancy + # delta --dark --paging=never + # ydiff -p cat -s --wrap --width={{columnWidth}} + pager: "" + + useConfig: false + + # e.g. 'difft --color=always' + externalDiffCommand: "" + + # Config relating to committing + commit: + # If true, pass '--signoff' flag when committing + signOff: false + + # Automatic WYSIWYG wrapping of the commit message as you type + autoWrapCommitMessage: true + + # If autoWrapCommitMessage is true, the width to wrap to + autoWrapWidth: 72 + + # Config relating to merging + merging: + # If true, run merges in a subprocess so that if a commit message is required, Lazygit will not hang + # Only applicable to unix users. + manualCommit: false + + # Extra args passed to , e.g. --no-ff + args: "" + + # The commit message to use for a squash merge commit. Can contain "{{selectedRef}}" and "{{currentBranch}}" placeholders. + squashMergeMessage: Squash merge {{selectedRef}} into {{currentBranch}} + + # list of branches that are considered 'main' branches, used when displaying commits + mainBranches: + - master + - main + + # Prefix to use when skipping hooks. E.g. if set to 'WIP', then pre-commit hooks will be skipped when the commit message starts with 'WIP' + skipHookPrefix: WIP + + # If true, periodically fetch from remote + autoFetch: true + + # If true, periodically refresh files and submodules + autoRefresh: true + + # If true, pass the --all arg to git fetch + fetchAll: true + + # If true, lazygit will automatically stage files that used to have merge + # conflicts but no longer do; and it will also ask you if you want to + # continue a merge or rebase if you've resolved all conflicts. If false, it + # won't do either of these things. + autoStageResolvedConflicts: true + + # Command used when displaying the current branch git log in the main window + branchLogCmd: git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} -- + + # Command used to display git log of all branches in the main window. + # Deprecated: Use allBranchesLogCmds instead. + allBranchesLogCmd: git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium + + # If true, do not spawn a separate process when using GPG + overrideGpg: false + + # If true, do not allow force pushes + disableForcePushing: false + + # See https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#predefined-branch-name-prefix + branchPrefix: "" + + # If true, parse emoji strings in commit messages e.g. render :rocket: as 🚀 + # (This should really be under 'gui', not 'git') + parseEmoji: false + + # Config for showing the log in the commits view + log: + # One of: 'date-order' | 'author-date-order' | 'topo-order' | 'default' + # 'topo-order' makes it easier to read the git log graph, but commits may not + # appear chronologically. See https://git-scm.com/docs/ + # + # Deprecated: Configure this with Log menu -> Commit sort order ( in the commits window by default). + order: topo-order + + # This determines whether the git graph is rendered in the commits panel + # One of 'always' | 'never' | 'when-maximised' + # + # Deprecated: Configure this with Log menu -> Show git graph ( in the commits window by default). + showGraph: always + + # displays the whole git graph by default in the commits view (equivalent to passing the --all argument to git log) + showWholeGraph: false + + # When copying commit hashes to the clipboard, truncate them to this + # length. Set to 40 to disable truncation. + truncateCopiedCommitHashesTo: 12 + +# Periodic update checks +update: + # One of: 'prompt' (default) | 'background' | 'never' + method: prompt + + # Period in days between update checks + days: 14 + +# Background refreshes +refresher: + # File/submodule refresh interval in seconds. + # Auto-refresh can be disabled via option 'git.autoRefresh'. + refreshInterval: 10 + + # Re-fetch interval in seconds. + # Auto-fetch can be disabled via option 'git.autoFetch'. + fetchInterval: 60 + +# If true, show a confirmation popup before quitting Lazygit +confirmOnQuit: false + +# If true, exit Lazygit when the user presses escape in a context where there is nothing to cancel/close +quitOnTopLevelReturn: false + +# Config relating to things outside of Lazygit like how files are opened, copying to clipboard, etc +os: + # Command for editing a file. Should contain "{{filename}}". + edit: "" + + # Command for editing a file at a given line number. Should contain + # "{{filename}}", and may optionally contain "{{line}}". + editAtLine: "" + + # Same as EditAtLine, except that the command needs to wait until the + # window is closed. + editAtLineAndWait: "" + + # Whether lazygit suspends until an edit process returns + editInTerminal: false + + # For opening a directory in an editor + openDirInEditor: "" + + # A built-in preset that sets all of the above settings. Supported presets + # are defined in the getPreset function in editor_presets.go. + editPreset: "" + + # Command for opening a file, as if the file is double-clicked. Should + # contain "{{filename}}", but doesn't support "{{line}}". + open: "" + + # Command for opening a link. Should contain "{{link}}". + openLink: "" + + # EditCommand is the command for editing a file. + # Deprecated: use Edit instead. Note that semantics are different: + # EditCommand is just the command itself, whereas Edit contains a + # "{{filename}}" variable. + editCommand: "" + + # EditCommandTemplate is the command template for editing a file + # Deprecated: use EditAtLine instead. + editCommandTemplate: "" + + # OpenCommand is the command for opening a file + # Deprecated: use Open instead. + openCommand: "" + + # OpenLinkCommand is the command for opening a link + # Deprecated: use OpenLink instead. + openLinkCommand: "" + + # CopyToClipboardCmd is the command for copying to clipboard. + # See https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#custom-command-for-copying-to-and-pasting-from-clipboard + copyToClipboardCmd: "" + + # ReadFromClipboardCmd is the command for reading the clipboard. + # See https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#custom-command-for-copying-to-and-pasting-from-clipboard + readFromClipboardCmd: "" + +# If true, don't display introductory popups upon opening Lazygit. +disableStartupPopups: false + +# What to do when opening Lazygit outside of a git repo. +# - 'prompt': (default) ask whether to initialize a new repo or open in the most recent repo +# - 'create': initialize a new repo +# - 'skip': open most recent repo +# - 'quit': exit Lazygit +notARepository: prompt + +# If true, display a confirmation when subprocess terminates. This allows you to view the output of the subprocess before returning to Lazygit. +promptToReturnFromSubprocess: true + +# Keybindings +keybinding: + universal: + quit: q + quit-alt1: + return: + quitWithoutChangingDirectory: Q + togglePanel: + prevItem: + nextItem: + prevItem-alt: k + nextItem-alt: j + prevPage: ',' + nextPage: . + scrollLeft: H + scrollRight: L + gotoTop: < + gotoBottom: '>' + toggleRangeSelect: v + rangeSelectDown: + rangeSelectUp: + prevBlock: + nextBlock: + prevBlock-alt: h + nextBlock-alt: l + nextBlock-alt2: + prevBlock-alt2: + jumpToBlock: + - "1" + - "2" + - "3" + - "4" + - "5" + nextMatch: "n" + prevMatch: "N" + startSearch: / + optionMenu: + optionMenu-alt1: '?' + select: + goInto: + confirm: + confirmInEditor: + remove: d + new: "n" + edit: e + openFile: o + scrollUpMain: + scrollDownMain: + scrollUpMain-alt1: K + scrollDownMain-alt1: J + scrollUpMain-alt2: + scrollDownMain-alt2: + executeShellCommand: ':' + createRebaseOptionsMenu: m + + # 'Files' appended for legacy reasons + pushFiles: P + + # 'Files' appended for legacy reasons + pullFiles: p + refresh: R + createPatchOptionsMenu: + nextTab: ']' + prevTab: '[' + nextScreenMode: + + prevScreenMode: _ + undo: z + redo: + filteringMenu: + diffingMenu: W + diffingMenu-alt: + copyToClipboard: + openRecentRepos: + submitEditorText: + extrasMenu: '@' + toggleWhitespaceInDiffView: + increaseContextInDiffView: '}' + decreaseContextInDiffView: '{' + increaseRenameSimilarityThreshold: ) + decreaseRenameSimilarityThreshold: ( + openDiffTool: + status: + checkForUpdate: u + recentRepos: + allBranchesLogGraph: a + files: + commitChanges: c + commitChangesWithoutHook: w + amendLastCommit: A + commitChangesWithEditor: C + findBaseCommitForFixup: + confirmDiscard: x + ignoreFile: i + refreshFiles: r + stashAllChanges: s + viewStashOptions: S + toggleStagedAll: a + viewResetOptions: D + fetch: f + openMergeTool: M + openStatusFilter: + copyFileInfoToClipboard: "y" + collapseAll: '-' + expandAll: = + branches: + createPullRequest: o + viewPullRequestOptions: O + copyPullRequestURL: + checkoutBranchByName: c + forceCheckoutBranch: F + rebaseBranch: r + renameBranch: R + mergeIntoCurrentBranch: M + viewGitFlowOptions: i + fastForward: f + createTag: T + pushTag: P + setUpstream: u + fetchRemote: f + sortOrder: s + worktrees: + viewWorktreeOptions: w + commits: + squashDown: s + renameCommit: r + renameCommitWithEditor: R + viewResetOptions: g + markCommitAsFixup: f + createFixupCommit: F + squashAboveCommits: S + moveDownCommit: + moveUpCommit: + amendToCommit: A + resetCommitAuthor: a + pickCommit: p + revertCommit: t + cherryPickCopy: C + pasteCommits: V + markCommitAsBaseForRebase: B + tagCommit: T + checkoutCommit: + resetCherryPick: + copyCommitAttributeToClipboard: "y" + openLogMenu: + openInBrowser: o + viewBisectOptions: b + startInteractiveRebase: i + amendAttribute: + resetAuthor: a + setAuthor: A + addCoAuthor: c + stash: + popStash: g + renameStash: r + commitFiles: + checkoutCommitFile: c + main: + toggleSelectHunk: a + pickBothHunks: b + editSelectHunk: E + submodules: + init: i + update: u + bulkMenu: b + commitMessage: + commitMenu: +`) + +func BenchmarkMigrationOnLargeConfiguration(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = computeMigratedConfig("path doesn't matter", largeConfiguration) + } +} diff --git a/pkg/utils/yaml_utils/yaml_utils.go b/pkg/utils/yaml_utils/yaml_utils.go index 6e031413e..956805691 100644 --- a/pkg/utils/yaml_utils/yaml_utils.go +++ b/pkg/utils/yaml_utils/yaml_utils.go @@ -35,7 +35,7 @@ func UpdateYamlValue(yamlBytes []byte, path []string, value string) ([]byte, err } // Convert the updated YAML node back to YAML bytes. - updatedYAMLBytes, err := yamlMarshal(body) + updatedYAMLBytes, err := YamlMarshal(body) if err != nil { return nil, fmt.Errorf("failed to convert YAML node to bytes: %w", err) } @@ -100,147 +100,101 @@ func lookupKey(node *yaml.Node, key string) (*yaml.Node, *yaml.Node) { return nil, nil } -// Walks a yaml document to the specified path, and then applies the transformation to that node. -// -// The transform must return true if it made changes to the node. +// Walks a yaml document from the root node to the specified path, and then applies the transformation to that node. // If the requested path is not defined in the document, no changes are made to the document. -// -// If no changes are made, the original document is returned. -// If changes are made, a newly marshalled document is returned. (This may result in different indentation for all nodes) -func TransformNode(yamlBytes []byte, path []string, transform func(node *yaml.Node) (bool, error)) ([]byte, error) { - // Parse the YAML file. - var node yaml.Node - err := yaml.Unmarshal(yamlBytes, &node) - if err != nil { - return nil, fmt.Errorf("failed to parse YAML: %w", err) - } - +func TransformNode(rootNode *yaml.Node, path []string, transform func(node *yaml.Node) error) error { // Empty document: nothing to do. - if len(node.Content) == 0 { - return yamlBytes, nil + if len(rootNode.Content) == 0 { + return nil } - body := node.Content[0] + body := rootNode.Content[0] - if didTransform, err := transformNode(body, path, transform); err != nil || !didTransform { - return yamlBytes, err + if err := transformNode(body, path, transform); err != nil { + return err } - // Convert the updated YAML node back to YAML bytes. - updatedYAMLBytes, err := yamlMarshal(body) - if err != nil { - return nil, fmt.Errorf("failed to convert YAML node to bytes: %w", err) - } - - return updatedYAMLBytes, nil + return nil } // A recursive function to walk down the tree. See TransformNode for more details. -func transformNode(node *yaml.Node, path []string, transform func(node *yaml.Node) (bool, error)) (bool, error) { +func transformNode(node *yaml.Node, path []string, transform func(node *yaml.Node) error) error { if len(path) == 0 { return transform(node) } keyNode, valueNode := lookupKey(node, path[0]) if keyNode == nil { - return false, nil + return nil } return transformNode(valueNode, path[1:], transform) } -// takes a yaml document in bytes, a path to a key, and a new name for the key. +// Takes the root node of a yaml document, a path to a key, and a new name for the key. // Will rename the key to the new name if it exists, and do nothing otherwise. -func RenameYamlKey(yamlBytes []byte, path []string, newKey string) ([]byte, error) { - // Parse the YAML file. - var node yaml.Node - err := yaml.Unmarshal(yamlBytes, &node) - if err != nil { - return nil, fmt.Errorf("failed to parse YAML: %w for bytes %s", err, string(yamlBytes)) - } - +func RenameYamlKey(rootNode *yaml.Node, path []string, newKey string) error { // Empty document: nothing to do. - if len(node.Content) == 0 { - return yamlBytes, nil + if len(rootNode.Content) == 0 { + return nil } - body := node.Content[0] + body := rootNode.Content[0] - if didRename, err := renameYamlKey(body, path, newKey); err != nil || !didRename { - return yamlBytes, err + if err := renameYamlKey(body, path, newKey); err != nil { + return err } - // Convert the updated YAML node back to YAML bytes. - updatedYAMLBytes, err := yamlMarshal(body) - if err != nil { - return nil, fmt.Errorf("failed to convert YAML node to bytes: %w", err) - } - - return updatedYAMLBytes, nil + return nil } // Recursive function to rename the YAML key. -func renameYamlKey(node *yaml.Node, path []string, newKey string) (bool, error) { +func renameYamlKey(node *yaml.Node, path []string, newKey string) error { if node.Kind != yaml.MappingNode { - return false, errors.New("yaml node in path is not a dictionary") + return errors.New("yaml node in path is not a dictionary") } keyNode, valueNode := lookupKey(node, path[0]) if keyNode == nil { - return false, nil + return nil } // end of path reached: rename key if len(path) == 1 { // Check that new key doesn't exist yet if newKeyNode, _ := lookupKey(node, newKey); newKeyNode != nil { - return false, fmt.Errorf("new key `%s' already exists", newKey) + return fmt.Errorf("new key `%s' already exists", newKey) } keyNode.Value = newKey - return true, nil + return nil } return renameYamlKey(valueNode, path[1:], newKey) } // Traverses a yaml document, calling the callback function for each node. The -// callback is allowed to modify the node in place, in which case it should -// return true. The function returns the original yaml document if none of the -// callbacks returned true, and the modified document otherwise. -func Walk(yamlBytes []byte, callback func(node *yaml.Node, path string) bool) ([]byte, error) { - // Parse the YAML file. - var node yaml.Node - err := yaml.Unmarshal(yamlBytes, &node) - if err != nil { - return nil, fmt.Errorf("failed to parse YAML: %w", err) - } - +// callback is expected to modify the node in place +func Walk(rootNode *yaml.Node, callback func(node *yaml.Node, path string)) error { // Empty document: nothing to do. - if len(node.Content) == 0 { - return yamlBytes, nil + if len(rootNode.Content) == 0 { + return nil } - body := node.Content[0] + body := rootNode.Content[0] - if didChange, err := walk(body, "", callback); err != nil || !didChange { - return yamlBytes, err + if err := walk(body, "", callback); err != nil { + return err } - // Convert the updated YAML node back to YAML bytes. - updatedYAMLBytes, err := yamlMarshal(body) - if err != nil { - return nil, fmt.Errorf("failed to convert YAML node to bytes: %w", err) - } - - return updatedYAMLBytes, nil + return nil } -func walk(node *yaml.Node, path string, callback func(*yaml.Node, string) bool) (bool, error) { - didChange := callback(node, path) +func walk(node *yaml.Node, path string, callback func(*yaml.Node, string)) error { + callback(node, path) switch node.Kind { case yaml.DocumentNode: - return false, errors.New("Unexpected document node in the middle of a yaml tree") + return errors.New("Unexpected document node in the middle of a yaml tree") case yaml.MappingNode: for i := 0; i < len(node.Content); i += 2 { name := node.Content[i].Value @@ -251,31 +205,29 @@ func walk(node *yaml.Node, path string, callback func(*yaml.Node, string) bool) } else { childPath = fmt.Sprintf("%s.%s", path, name) } - didChangeChild, err := walk(childNode, childPath, callback) + err := walk(childNode, childPath, callback) if err != nil { - return false, err + return err } - didChange = didChange || didChangeChild } case yaml.SequenceNode: for i := 0; i < len(node.Content); i++ { childPath := fmt.Sprintf("%s[%d]", path, i) - didChangeChild, err := walk(node.Content[i], childPath, callback) + err := walk(node.Content[i], childPath, callback) if err != nil { - return false, err + return err } - didChange = didChange || didChangeChild } case yaml.ScalarNode: // nothing to do case yaml.AliasNode: - return false, errors.New("Alias nodes are not supported") + return errors.New("Alias nodes are not supported") } - return didChange, nil + return nil } -func yamlMarshal(node *yaml.Node) ([]byte, error) { +func YamlMarshal(node *yaml.Node) ([]byte, error) { var buffer bytes.Buffer encoder := yaml.NewEncoder(&buffer) encoder.SetIndent(2) diff --git a/pkg/utils/yaml_utils/yaml_utils_test.go b/pkg/utils/yaml_utils/yaml_utils_test.go index 21433d32d..e53bb1354 100644 --- a/pkg/utils/yaml_utils/yaml_utils_test.go +++ b/pkg/utils/yaml_utils/yaml_utils_test.go @@ -186,14 +186,16 @@ func TestRenameYamlKey(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - out, actualErr := RenameYamlKey([]byte(test.in), test.path, test.newKey) + node := unmarshalForTest(t, test.in) + actualErr := RenameYamlKey(&node, test.path, test.newKey) if test.expectedErr == "" { assert.NoError(t, actualErr) } else { assert.EqualError(t, actualErr, test.expectedErr) } + out := marshalForTest(t, &node) - assert.Equal(t, test.expectedOut, string(out)) + assert.Equal(t, test.expectedOut, out) }) } } @@ -238,10 +240,10 @@ func TestWalk_paths(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + node := unmarshalForTest(t, test.document) paths := []string{} - _, err := Walk([]byte(test.document), func(node *yaml.Node, path string) bool { + err := Walk(&node, func(node *yaml.Node, path string) { paths = append(paths, path) - return true }) assert.NoError(t, err) @@ -254,48 +256,41 @@ func TestWalk_inPlaceChanges(t *testing.T) { tests := []struct { name string in string - callback func(node *yaml.Node, path string) bool + callback func(node *yaml.Node, path string) expectedOut string }{ { - name: "no change", - in: "x: 5", - callback: func(node *yaml.Node, path string) bool { return false }, - expectedOut: "x: 5", + name: "no change", + in: "x: 5", + callback: func(node *yaml.Node, path string) {}, }, { name: "change value", in: "x: 5\ny: 3", - callback: func(node *yaml.Node, path string) bool { + callback: func(node *yaml.Node, path string) { if path == "x" { node.Value = "7" - return true } - return false }, expectedOut: "x: 7\ny: 3\n", }, { name: "change nested value", in: "x:\n y: 5", - callback: func(node *yaml.Node, path string) bool { + callback: func(node *yaml.Node, path string) { if path == "x.y" { node.Value = "7" - return true } - return false }, expectedOut: "x:\n y: 7\n", }, { name: "change array value", in: "x:\n - y: 5", - callback: func(node *yaml.Node, path string) bool { + callback: func(node *yaml.Node, path string) { if path == "x[0].y" { node.Value = "7" - return true } - return false }, expectedOut: "x:\n - y: 7\n", }, @@ -303,28 +298,34 @@ func TestWalk_inPlaceChanges(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - result, err := Walk([]byte(test.in), test.callback) - + node := unmarshalForTest(t, test.in) + err := Walk(&node, test.callback) assert.NoError(t, err) - assert.Equal(t, test.expectedOut, string(result)) + if test.expectedOut == "" { + unmodifiedOriginal := unmarshalForTest(t, test.in) + assert.Equal(t, unmodifiedOriginal, node) + } else { + result := marshalForTest(t, &node) + assert.Equal(t, test.expectedOut, result) + } }) } } func TestTransformNode(t *testing.T) { - transformIntValueToString := func(node *yaml.Node) (bool, error) { + transformIntValueToString := func(node *yaml.Node) error { if node.Kind == yaml.ScalarNode { if node.ShortTag() == "!!int" { node.Tag = "!!str" - return true, nil + return nil } else if node.ShortTag() == "!!str" { // We have already transformed it, - return false, nil + return nil } else { - return false, fmt.Errorf("Node was of bad type") + return fmt.Errorf("Node was of bad type") } } else { - return false, fmt.Errorf("Node was not a scalar") + return fmt.Errorf("Node was not a scalar") } } @@ -332,15 +333,14 @@ func TestTransformNode(t *testing.T) { name string in string path []string - transform func(node *yaml.Node) (bool, error) + transform func(node *yaml.Node) error expectedOut string }{ { - name: "Path not present", - in: "foo: 1", - path: []string{"bar"}, - transform: transformIntValueToString, - expectedOut: "foo: 1", + name: "Path not present", + in: "foo: 1", + path: []string{"bar"}, + transform: transformIntValueToString, }, { name: "Part of path present", @@ -349,9 +349,6 @@ foo: bar: 2`, path: []string{"foo", "baz"}, transform: transformIntValueToString, - expectedOut: ` -foo: - bar: 2`, }, { name: "Successfully Transforms to string", @@ -371,19 +368,42 @@ foo: bar: "2"`, path: []string{"foo", "bar"}, transform: transformIntValueToString, - expectedOut: ` -foo: - bar: "2"`, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - result, err := TransformNode([]byte(test.in), test.path, test.transform) + node := unmarshalForTest(t, test.in) + err := TransformNode(&node, test.path, test.transform) if err != nil { t.Fatal(err) } - assert.Equal(t, test.expectedOut, string(result)) + if test.expectedOut == "" { + unmodifiedOriginal := unmarshalForTest(t, test.in) + assert.Equal(t, unmodifiedOriginal, node) + } else { + result := marshalForTest(t, &node) + assert.Equal(t, test.expectedOut, result) + } }) } } + +func unmarshalForTest(t *testing.T, input string) yaml.Node { + t.Helper() + var node yaml.Node + err := yaml.Unmarshal([]byte(input), &node) + if err != nil { + t.Fatal(err) + } + return node +} + +func marshalForTest(t *testing.T, node *yaml.Node) string { + t.Helper() + result, err := YamlMarshal(node) + if err != nil { + t.Fatal(err) + } + return string(result) +}