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

Use $XDG_STATE_HOME to store history.db #349

Closed
wants to merge 2 commits into from

Conversation

utkarshgupta137
Copy link
Contributor

@utkarshgupta137 utkarshgupta137 commented Apr 16, 2023

Fixes #243 #269.

@utkarshgupta137 utkarshgupta137 changed the title l Use $XDG_STATE_HOME to store history.db Apr 16, 2023
@utkarshgupta137 utkarshgupta137 marked this pull request as ready for review April 16, 2023 05:09
let cache_dir = Settings::mcfly_xdg_dir().cache_dir().to_path_buf();

Settings::mcfly_base_path(cache_dir).join(PathBuf::from("training-cache.v1.csv"))
Settings::xdg_base_strategy()
Copy link
Owner

Choose a reason for hiding this comment

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

What is the difference between mcfly_xdg_dir and xdg_base_strategy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could reuse the name if you want.

Copy link
Owner

Choose a reason for hiding this comment

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

I just didn't understand what the difference is between them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mcfly_xdg_dir returned a ProjectDirs from the directories crate. xdg_base_strategy base directory returns an Xdg struct from etcetera crate which is the counterpart to it.

src/settings.rs Outdated
@@ -355,35 +356,51 @@ impl Settings {
settings
}

// Use ~/.mcfly only if it already exists, otherwise create 'mcfly' folder in XDG_CACHE_DIR
// Use or create the 'mcfly' folder in `$XDG_CACHE_DIR``
Copy link
Owner

Choose a reason for hiding this comment

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

Two ``

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

src/settings.rs Outdated

fn mcfly_xdg_dir() -> ProjectDirs {
ProjectDirs::from("", "", "McFly").unwrap()
let path = Settings::xdg_base_strategy()
Copy link
Owner

Choose a reason for hiding this comment

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

Is this backwards compatible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, forgot to migrate from data_dir, that's done now.

@utkarshgupta137
Copy link
Contributor Author

@cantino ping

@@ -341,7 +341,10 @@ Note that only single-character-prompts are allowed. setting `MCFLY_PROMPT` to `

### Database Location

McFly stores its SQLite database in the standard location for the OS. On OS X, this is in `~/Library/Application Support/McFly` and on Linux it is in `$XDG_DATA_DIR/mcfly/history.db` (default would be `~/.local/share/mcfly/history.db`). For legacy support, if `~/.mcfly/` exists, it is used instead.
Copy link
Owner

Choose a reason for hiding this comment

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

I think instead of migrating, it should keep using the old location if it exists.

@o6uoq
Copy link

o6uoq commented Apr 22, 2024

This PR has been open since April 2023 - is there an update if this is still going to be baked into McFly?

@praveenperera
Copy link
Collaborator

Closed by: #244

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.

Make McFly history.db path configurable
4 participants