Skip to content

Fix#275

Merged
MordechaiHadad merged 2 commits into
masterfrom
fix/erase-cmd
Jun 8, 2025
Merged

Fix#275
MordechaiHadad merged 2 commits into
masterfrom
fix/erase-cmd

Conversation

@MordechaiHadad

@MordechaiHadad MordechaiHadad commented May 4, 2025

Copy link
Copy Markdown
Owner

Fix incorrect order of executions found by @daeho-ro

@MordechaiHadad MordechaiHadad requested a review from Copilot May 4, 2025 15:08
Copilot AI reviewed May 4, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses the issue of incorrect execution order for erasing files and directories as identified by @daeho-ro. Key changes include reordering the file removal logic in the erasure handler, fixing the use of the downloads directory in the shell branch, and removing an obsolete configuration file along with an unused property in Cargo.toml.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/handlers/erase_handler.rs Reorders removal logic and adjusts the env_path assignment
cliff.toml Removes deprecated git-cliff configuration
Cargo.toml Removes an unused "plugin" property from the binary configuration

Comment thread src/handlers/erase_handler.rs
@daeho-ro daeho-ro mentioned this pull request May 4, 2025
Closed
@daeho-ro

daeho-ro commented May 4, 2025

Copy link
Copy Markdown

We have another error,

  ==> /opt/homebrew/Cellar/bob/4.1.0/bin/bob list
  ==> /opt/homebrew/Cellar/bob/4.1.0/bin/bob erase
  May 04 11:27:36.485 ERROR Error: Home environment and ZDOTDIR variables are empty
  Error: bob: failed

It might be looking for some shell envs but cannot found.
Maybe it is assumed that some rc file changes but I think this should be fallback not cause an error.

@MordechaiHadad

MordechaiHadad commented May 4, 2025

Copy link
Copy Markdown
Owner Author

We have another error,

  ==> /opt/homebrew/Cellar/bob/4.1.0/bin/bob list
  ==> /opt/homebrew/Cellar/bob/4.1.0/bin/bob erase
  May 04 11:27:36.485 ERROR Error: Home environment and ZDOTDIR variables are empty
  Error: bob: failed

It might be looking for some shell envs but cannot found. Maybe it is assumed that some rc file changes but I think this should be fallback not cause an error.

Hmm I will try to check this tomorrow, do $HOME or $ZDOTDIR not exist on macos?

@daeho-ro

daeho-ro commented May 4, 2025

Copy link
Copy Markdown

I think so, it is from github actions.

@MordechaiHadad

MordechaiHadad commented May 5, 2025

Copy link
Copy Markdown
Owner Author

I think so, it is from github actions.

That is odd then, $ZDOTDIR is the way zsh determines the location of the zshrc and the primary fallback for it is inside $HOME, so I am a bit confused here.

@MordechaiHadad

MordechaiHadad commented May 8, 2025

Copy link
Copy Markdown
Owner Author

@daeho-ro how is it resolvable? I want to publish the release... I don't know how Github Actions macos works but this is like the only way to retrieve the user's zshrc file that I know of.

@daeho-ro

daeho-ro commented May 8, 2025

Copy link
Copy Markdown

I think need some time to dig more, homebrew installation is somehow wrong. You can still release whenever, without version bump on the homebrew side.

@MordechaiHadad

MordechaiHadad commented May 8, 2025

Copy link
Copy Markdown
Owner Author

I think need some time to dig more, homebrew installation is somehow wrong. You can still release whenever, without version bump on the homebrew side.

Hmm I see, keep me posted.

@daeho-ro

daeho-ro commented May 8, 2025 •
edited
Loading

Copy link
Copy Markdown

Homebrew only did cargo install and if I run bob erase without any configuration,

> bob erase
May 09 01:15:05.310 ERROR Error: Home environment and ZDOTDIR variables are empty

I got this error. I am using warp terminal with zsh. I have HOME but not ZDOTDIR.

> echo $HOME
/Users/daeho.ro
> echo $ZDOTDIR

Am I miss something to install or any further installation steps?

@MordechaiHadad

MordechaiHadad commented May 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Owner Author

Hmmm it's giving an error that $home is empty despite it not? I'll try to fix this.

Edit: you are also probably not missing anything in the installation steps it's probably something I fucked up.

@MordechaiHadad

MordechaiHadad commented May 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Owner Author

@daeho-ro okay i think the problem might be cuz i am only modifying .zshenv, i will add .zshrc as last fallback and we will see if it resolves it.

Or I can just ignore those errors and skip anything related to path erasure

@MordechaiHadad

MordechaiHadad commented Jun 1, 2025

Copy link
Copy Markdown
Owner Author

yo @daeho-ro sorry for hiatus, will this do? I decided to just ignore the errors and skip path erasure

@MordechaiHadad MordechaiHadad merged commit 110de01 into master Jun 8, 2025
53 checks passed
@MordechaiHadad MordechaiHadad deleted the fix/erase-cmd branch June 8, 2025 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MordechaiHadad @daeho-ro