Skip to content
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

fix: allow config file to load showLineNumbers #653

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

prithvijj
Copy link

@prithvijj prithvijj commented Oct 16, 2024

Changes

  • Added in a fix to allow the showLineNumbers config option to
    be loaded from the config file
  • Previously it seems unclear on how to setup the showLineNumbers key
    within the config, and while there was support added for configuring
    the showLineNumbers flag, it couldn't be read from the config file,
    since the validateOptions method would not retrieve the key from
    the config file
  • Hence changed it so that it does retrieve the showLineNumbers key
    from the config file, and use it as needed
  • Updated the README.md to also provide an example on how to setup
    both the keys

Testing Notes

Ran

go build
./glow config

# added in `preserveNewLines: true` and `showLineNumbers: true` key 

and for testing purposes to see if the config option was being read added in

func validateOptions(cmd *cobra.Command) error {
	// grab config values from Viper
	width = viper.GetUint("width")
	mouse = viper.GetBool("mouse")
	pager = viper.GetBool("pager")
	showAllFiles = viper.GetBool("all")
	preserveNewLines = viper.GetBool("preserveNewLines")

+	panic(fmt.Sprintf("preserveNewLines: %v, showLineNumbers: %v", preserveNewLines, showLineNumbers))

to showcase the configurations are being read

Previously:
When preserveNewLines: true and showLineNumbers: true within the config, the panic output shows that
preserveNewLines is being recognized, but showLineNumbers is not being recognized, which is unexpected

image

When ./glow -l is used, the panic output shows that the preserveNewLines is being recognized, and showLineNumbers is being recognized which is expected

image


After:

When preserveNewLines: true and showLineNumbers: true within the config, the panic output shows that
preserveNewLines is being recognized and showLineNumbers is being recognized, which is expected
image

When preserveNewLines: true and showLineNumbers: false within the config, the panic output shows that
preserveNewLines is being recognized and showLineNumbers is being recognized, which is expected
image

Removed the panic line used for testing purposes, ran go build; ./glow then navigated to the README.md to show that the line numbers are being shown, since the config has showLineNumbers: true
image

Other Notes

Related to #652

  • Any tests that I can write?

- Added in a fix to allow the `showLineNumbers` config option to
  be loaded from the config file
- Previously it seems unclear on how to setup the `showLineNumbers` key
  within the config, and while there was support added for configuring
  the `showLineNumbers` flag, it couldn't be read from the config file,
  since the `validateOptions` method would not retrieve the key from
  the config file
- Hence changed it so that it does retrieve the `showLineNumbers` key
  from the config file, and use it as needed
- Updated the README.md to also provide an example on how to setup
  both the keys
@prithvijj prithvijj changed the title fix: allow config file to load line-numbers and preserve-new-lines fix: allow config file to load showLineNumbers Oct 16, 2024
@prithvijj prithvijj marked this pull request as ready for review October 16, 2024 14:13
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