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

Add support for ~/.config/lazygit without setting XDG_CONFIG_HOME on macOS #3989

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

BlakeWilliams
Copy link

@BlakeWilliams BlakeWilliams commented Oct 12, 2024

Currently lazygit looks for its config file in XDG_CONFIG_HOME if it's available, but if not it falls back to the defaults defined by the xdg package. Unfortunately the defaults the package falls back to isn't what CLI applications commonly fall back to on macOS. Specifically, it looks in ~/Library/Application Support instead of ~/.config.

This updates the app config logic to:

  • Look for ~/.config/lazygit first if XDG_CONFIG_HOME is not set and we're on macOS.
  • Fallback to the existing xdg package location if the configuration file exists there.
  • Default to ~/.config/lazygit/config.yml if XDG_CONFIG_HOME is not set, we're on macOS, and there is no existing configuration file.

This change did feel a bit like having to thread a needle and I didn't see any existing tests for this behavior (which is reasonable, since it's complicated and OS dependent) so I did test a few variations of the configuration locally by building with this change included and comparing against a brew installed lazygit.

It seemed to work properly, falling back to the existing location when XDG_CONFIG_HOME isn't set, using ~/.config/lazygit when config.yml is present, and creating ~/.config/lazygit/config.yml when it's not.

I think this should resolve #1341

  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
  • Docs have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

Currently lazygit looks for its config file in `XDG_CONFIG_HOME` if it's
available, but if not it falls back to the defaults defined by the
[xdg](https://github.com/adrg/xdg) package. Unfortunately the defaults
the package falls back to isn't what CLI applications commonly fall back
to on macOS. Specifically, it looks in `~/Library/Application Support`
instead of `~/.config`.

This updates the app config logic to:

- Look for `~/.config/lazygit` first if `XDG_CONFIG_HOME` is not set
  and we're on macOS.
- Fallback to the existing `xdg` package location if the configuration
  file exists there.
- Default to `~/.config/lazygit/config.yml` if `XDG_CONFIG_HOME` is not
  set, we're on macOS, and there is no existing configuration file.

This change did feel a bit like having to thread a needle and I didn't
see any existing tests for this behavior (which is reasonable, since it's
complicated and OS dependent) so I did test a few variations of the
configuration locally by building with this change included and running
a `brew` installed lazygit.

It seemed to work properly, falling back to the existing location when
`XDG_CONFIG_HOME` isn't set, using `~/.config/lazygit` when `config.yml`
is present, and creating `~/.config/lazygit/config.yml` when it's not.

I think this should resolve jesseduffield#1341
@BlakeWilliams
Copy link
Author

I took a quick pass at updating the docs, but I didn't want to change too much without getting thoughts on how we might want to communicate this change, if at all. Happy to update that if you have any preferences there!

@BlakeWilliams BlakeWilliams marked this pull request as ready for review October 12, 2024 19:35
@@ -349,13 +349,34 @@ func findConfigFile(filename string) (exists bool, path string) {
return true, legacyConfigPath
}

// if on macOs, default to looking in ~/.config/lazygit first since that's
Copy link
Author

@BlakeWilliams BlakeWilliams Oct 12, 2024

Choose a reason for hiding this comment

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

I generally don't like lengthy comments in the middle of methods, but I thought that this was obtuse enough to warrant a description of why we have to implement this behavior instead of using xdg.

Copy link
Owner

@jesseduffield jesseduffield left a comment

Choose a reason for hiding this comment

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

LGTM. Just so there's no ambiguity: if I've got my current config file in ~/Library/Application\ Support/lazygit/config.yml and I don't have the xdg env var set, lazygit will continue using the config from ~/Library/Application\ Support/lazygit/config.yml?

@BlakeWilliams
Copy link
Author

LGTM. Just so there's no ambiguity: if I've got my current config file in ~/Library/Application\ Support/lazygit/config.yml and I don't have the xdg env var set, lazygit will continue using the config from ~/Library/Application\ Support/lazygit/config.yml?

Just double checked this and output some debug logs for additional confidence:

With XDG_CONFIG_HOME unset, and ~/.config/lazygit/config.yml exists:

lazygit ❯ go run .
&{/Users/Blake/.config/lazygit/config.yml 0 {339739848 63870478900 0x105f1f020} true}

With XDG_CONFIG_HOME unset, and ~/.config/lazygit/config.yml does not exist:

lazygit ❯ go run .
&{/Users/Blake/Library/Application Support/lazygit/config.yml 0 {339739848 63870478900 0x101f87020} true}

So yes, you're correct that it will fall back to ~/Library/Application\ Support/lazygit/config.yml when XDG_CONFIG_HOME is unset and ~/.config/lazygit/config.yml does not exist.

@jesseduffield jesseduffield added the enhancement New feature or request label Dec 31, 2024
@jesseduffield
Copy link
Owner

Cool, I'm happy to merge this, but can I get you to squash your changes into a single commit? Given you've merged the master branch into your branch, this might be messy. Here's a script I use to disentangle things in that state:

#!/bin/bash

# Set the base branch
BASE_BRANCH="master"

# Check if the working tree is dirty
if [[ -n $(git status --porcelain) ]]; then
    echo "Error: Working tree is dirty. Please commit or stash your changes before running this script."
    exit 1
fi

# Get the merge base commit
merge_base=$(git merge-base $BASE_BRANCH HEAD)

# Get the first commit hash, message, and author details
first_commit_hash=$(git rev-list --reverse $merge_base..HEAD | head -n 1)
first_commit_message=$(git log -1 --format=%B $first_commit_hash)

# Reset to the merge base
git reset $merge_base

# Stage all changes
git add -A

# Create a new commit with all the changes, using the first commit's message and author
GIT_AUTHOR_NAME="$(git log -1 --format='%an' $first_commit_hash)" \
GIT_AUTHOR_EMAIL="$(git log -1 --format='%ae' $first_commit_hash)" \
git commit -m "$first_commit_message"

# Rebase onto the base branch
git rebase $BASE_BRANCH

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

Successfully merging this pull request may close these issues.

Support .config/XDG configuration location in MacOSX
2 participants