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

feat: add symlink support #193

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

boldandbrad
Copy link
Contributor

@boldandbrad boldandbrad commented Nov 25, 2024

Prerequisites

  • I have read and understood the contributing guide.
  • The commit message follows the conventional commits guidelines.
  • Tests for the changes have been added (for bug fixes / features).
  • Docs have been added/updated (for bug fixes / features).

I've built and tested within my own config using zsh on macOS.

Features

This PR closes #192.

  • Add symlink support, because dotfiles!

Now you can:

# .aliae.yaml
...

link:
  - name: ~/.aliae.yaml
    target: ~/dotfiles/aliae.yaml
  - name: ~/.zshrc
    target: $DOTFILES/config/zsh/zshrc
  - name: ~/Brewfile
    value: /some/location/Brewfile
    if: eq .OS "darwin"

Outstanding issues/considerations

I've defaulted to using -f (force) by default since it would be annoying to get notifications at every shell invocation about symlinks already existing. There's probably a better solution where there is a force attribute on Links that defaults to false but can be overridden, or a global force-links config. In addition, cmd does not have a -f option for mklink, so additional work might need to be done to remove and replace the symlink if force is actually wanted. I'd like some help determining which approach makes most sense for this tool.

Let me know what you think!

@boldandbrad
Copy link
Contributor Author

Rebased to main.

@JanDeDobbeleer
Copy link
Owner

@boldandbrad I will review later this week.

@boldandbrad
Copy link
Contributor Author

@JanDeDobbeleer I don't remember the linter being upset before I rebased to main (and the only thing that changed in main to my knowledge was the one-line rendering order that was merged in #196).

It appears to be upset that I "duplicated" the test setup from the alias_test file to test the symlink functionality.

It also wants me to create a constant for the ln -sf ... cmd I used for most of the implementations.

For both of these "errors" I want to make sure I'm following the patterns of the codebase, any suggestions to resolve?

No rush at all, I know you've been busy chasing omp install issues. I haven't had much time in the last week to address this either, but still interested in feedback when you get a chance.

@JanDeDobbeleer
Copy link
Owner

Still in my list, I'll still try to look this week!

@JanDeDobbeleer
Copy link
Owner

This week turns out to be this week, I'll merge it in the coming days with some adjustments.

@JanDeDobbeleer JanDeDobbeleer changed the base branch from main to next December 23, 2024 20:01
@JanDeDobbeleer JanDeDobbeleer force-pushed the feat/dotfiles branch 2 times, most recently from 739257b to 93842f5 Compare December 25, 2024 20:25
@JanDeDobbeleer JanDeDobbeleer force-pushed the feat/dotfiles branch 2 times, most recently from f2e2327 to 5b63028 Compare December 25, 2024 20:35
@JanDeDobbeleer JanDeDobbeleer force-pushed the feat/dotfiles branch 2 times, most recently from 27442e3 to 62efe7d Compare December 25, 2024 20:41
@JanDeDobbeleer
Copy link
Owner

I just need to test this and we're good to go

@JanDeDobbeleer JanDeDobbeleer merged commit 2e42f9d into JanDeDobbeleer:next Dec 27, 2024
4 checks passed
@JanDeDobbeleer
Copy link
Owner

@all-contributors please add @boldandbrad for doc

Copy link
Contributor

@JanDeDobbeleer

I've put up a pull request to add @boldandbrad! 🎉

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.

Symlink support
2 participants