10000 config: add GetOption and GetAllOptions to Sections and Subsections by davidalpert · Pull Request #1588 · go-git/go-git · GitHub
[go: up one dir, main page]

Skip to content

config: add GetOption and GetAllOptions to Sections and Subsections #1588

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

davidalpert
Copy link
Contributor

These config structures had HasOption, SetOption, and AddOption
but were missing GetOption and GetAllOptions even though the
underlying Option struct exposed Get and GetAll methods.

Echoing these up on the Sections and Subsections structs will
facilitate higher-level operations when reading config values.

Relates to: GH #395

@Copilot Copilot AI review requested due to automatic review settings June 19, 2025 21:11
Copilot

This comment was marked as outdated.

These config structures had HasOption, SetOption, and AddOption
but were missing GetOption and GetAllOptions even though the
underlying Option struct exposed Get and GetAll methods.

Echoing these up on the Sections and Subsections structs will
facilitate higher-level operations when reading config values.

Relates to: GH go-git#395
This behaviour is supported by Git CLI and also by the
underlying Options struct and was already exposed on the
Subsection.SetOption(..) method. This commit updates the
Section.SetOptions(..) signature to match and allow
easier setting of multiple values from the Section level.

Relates to: GH go-git#395
@davidalpert davidalpert force-pushed the 395-config-section-get-and-get-all branch from 22cf415 to 76a29b1 Compare June 19, 2025 22:03
@davidalpert davidalpert requested a review from Copilot June 25, 2025 15:56
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces retrieval methods for configuration options and updates option-setting APIs to handle multiple values.

  • Exposed GetOption and GetAllOptions on Section and Subsection
  • Added GetOption and GetAllOptions at the Config level
  • Changed SetOption methods to variadic signatures for multi-value support

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
plumbing/format/config/section.go Added GetOption/GetAllOptions, updated SetOption to variadic signature
plumbing/format/config/common.go Added Config.GetOption/GetAllOptions, updated SetOption to accept multiple values
plumbing/format/config/common_test.go New tests covering GetOption and GetAllOptions
Comments suppressed due to low confidence (5)

plumbing/format/config/section.go:130

  • Update the comment above this method to explain that multiple values can now be passed via the variadic value ...string parameter and describe how they are handled.
func (s *Section) SetOption(key string, value ...string) *Section {

plumbing/format/config/section.go:138

  • [nitpick] The phrase "Note that there is no difference." in the preceding comment is unclear—consider rewording or removing it to clarify the behavior when an option is missing or duplicated.
func (s *Section) GetOption(key string) string {

plumbing/format/config/common.go:124

  • The first line of this doc reads "gets an option" but this method returns all values; also the next line repeats the method name. Consider merging and correcting these lines into a single clear description.
// GetAllOptions gets an option from a given section and subsection. Use the

plumbing/format/config/common_test.go:141

  • This test is for GetAllOptions and returns an empty slice, not a string—update the comment to "return empty slice" for accuracy.
	// when option does not exist, return empty string

plumbing/format/config/common_test.go:160

  • The test for GetAllOptions expects all values, not just the last one—adjust the comment to say "return all values in order".
	// when multiple values exist, return last value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant
0