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

Document cli options that can be set in configuration #2693

Conversation

irkode
Copy link
Contributor

@irkode irkode commented Sep 5, 2024

implements Forum Topic 51064

  • add missing settings
  • fix sort order of headings in "All configuration settings"

@irkode
Copy link
Contributor Author

irkode commented Sep 5, 2024

Remark: regarding sort order of SectionAll configuration settings

It looks like all these list should be sorted which imho is a good setup.
Currently four entries are not at sort order location.

  • canonifyURLs
  • enableRobotsTXT
  • disableLanguages
  • removePathAccents

if you want I could add another commit to align the sort (or create another PR or leave as is) just as you prefer.

@jmooring
Copy link
Member

jmooring commented Sep 5, 2024

Thanks. Please rebase and make the sort corrections you mentioned.

@irkode irkode force-pushed the topic-51064-add-available-cli-options-to-config branch from 092596e to 56fd858 Compare September 5, 2024 18:40
@irkode irkode marked this pull request as ready for review September 5, 2024 18:42
@irkode
Copy link
Contributor Author

irkode commented Sep 5, 2024

rebased, sorted, updated PR and set to Review

content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
content/en/getting-started/configuration.md Outdated Show resolved Hide resolved
@irkode
Copy link
Contributor Author

irkode commented Sep 6, 2024

Thx for the review - I just took the stuff literally from the comandline help

just one suggestion:

What about using the phrase Whether to log WARNINGs when ... for the --print... flags

  • Would match the description in fmt.warnf.
  • imho makes using to the console superfluous

maybe without capitalizing warnings.

  • printI18nWarnings (this actually is a warning not just additional output)
    (bool) Whether to log WARNINGs for each missing translation. Default is false.

  • printPathWarnings
    (bool) Whether to log WARNINGs when Hugo publishes two or more files to the same path. Default is false.

  • printUnusedTemplates
    (bool) Whether to log WARNINGs for each unused template. Default is false.

if you don't like that, I'll take your proposed changes for the four open ones as is

@irkode irkode requested a review from jmooring September 6, 2024 06:38
@jmooring
Copy link
Member

jmooring commented Sep 6, 2024

What about using the phrase

Sure.

@jmooring
Copy link
Member

jmooring commented Sep 6, 2024

At this point I really don't care. Let's just get it done.

Fix sort order of options in section 'All configuration settings'

add changes requested by review

add changes requested by review as discussed
@irkode irkode force-pushed the topic-51064-add-available-cli-options-to-config branch from 199b78e to a8b3571 Compare September 6, 2024 20:21
@irkode
Copy link
Contributor Author

irkode commented Sep 6, 2024

hope that finally fits

@jmooring jmooring merged commit fd98a03 into gohugoio:master Sep 7, 2024
5 checks passed
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.

2 participants