feat(config-xdg): enable XDG conventions #12790
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
dbrtly
force-pushed
the
xdg
branch
2 times, most recently
from
96415c9 to
e05b9b6
Compare
packages/core/src/utils/xdg.ts
Outdated
| }; | ||
|
|
||
| export default function envPaths(name: string) { | ||
| if (typeof name !== 'string') { |
There was a problem hiding this comment.
don't think this is necessary as envPaths only accepts string
| return { | ||
| ...actual, | ||
| default: actual, | ||
| homedir: vi.fn(() => path.join('/mock', 'home')), |
There was a problem hiding this comment.
instead of doing this, can you capture the actual value and then use that to compare in the test? see below
| @@ -318,7 +324,12 @@ describe('MemoryTool', () => { | |||
| expect(result).not.toBe(false); | |||
|
|
|||
| if (result && result.type === 'edit') { | |||
| const expectedPath = path.join('~', GEMINI_DIR, 'GEMINI.md'); | |||
| const expectedPath = path.join( | |||
| '/mock', | |||
There was a problem hiding this comment.
capture expectedHome = os.homedir() then check that expectedPath starts with expectedHome
packages/core/src/utils/xdg.ts
Outdated
| }; | ||
| }; | ||
|
|
||
| export default function envPaths(name: string) { |
There was a problem hiding this comment.
Can you add a debugLogger statement here? I worry about users ending up intermittently using the XDG paths vs. homedir paths because XDG paths aren't used consistently, for example in cronjobs.
There was a problem hiding this comment.
Do we still need this with the reduced scope?
dbrtly
force-pushed
the
xdg
branch
2 times, most recently
from
524fe01 to
4c5cf7b
Compare
|
I reflected on the code and was unhappy about the complexity. Only XDG nerds are going to care about this and that the change should prioritise the experience of the majority of users. The XDG Base Directory Specification does not prescribe os-specific paths for windows or macos. So I reverted to simpler code that respects the XDG environment variables if they are set and if not set, ignores XDG continuing with the established practice of A consequence of this version is simpler debugging if some but not all env var are set. |
|
for config: |
|
Hi, I decided to test this today with |
Thanks @marmitar. I expanded the scope to every file that referenced GEMINI_DIR. Turns out that is quite a few. |
|
@cornmander after a break, have spotted a few things I have todo before ready for you. |
|
Somehow, there a failing tests on main. I introduced new functions And amend the code to use them. Will update the implementation of those with conditional logic in next PR and add tests on these functions |
5 participants
XDG configurations bring flexibility to files previously written only to
~/.gemini directory.Summary
Following XDG conventions for the path to gemini's configuration makes it easy to use Gemini as a developer tool while developing gemini and enables a clean home directory for users who like that.
Details
Preserved the existing behaviour while enabling compatibility with environment variables (eg. $XDG_CONFIG_HOME) on all platforms and if neither
~/.gemini/exists nor the environment variables are set, Gemini will use the platform-specific location for cache/config/data/state files.Related Issues
closes #1825
How to Validate
for config:
if
~/.gemini/exists and $XDG_CONFIG_HOME is not set then use~/.gemini/.if
~/.gemini/does not exist and $XDG_CONFIG_HOME is set then use$XDG_CONFIG_HOME/config/if
~/.gemini/does not exist and $XDG_CONFIG_HOME is not set then use platform-specific location.Pre-Merge Checklist