Skip to content
This repository was archived by the owner on Nov 1, 2024. It is now read-only.

Introduce applicationConfigHome #66

Merged
merged 2 commits into from
Sep 17, 2021

Conversation

jonasfj
Copy link
Contributor

@jonasfj jonasfj commented Sep 15, 2021

We started moving dart pub configuration files here, see: dart-lang/pub#2999 (comment)

I figured this logic might live in cli_utils, then it's easy to use in dartdev, etc...

Should also make it easy to fix dart-lang/sdk#41560

@jonasfj jonasfj requested a review from devoncarew September 15, 2021 13:58
@google-cla google-cla bot added the cla: yes label Sep 15, 2021
@devoncarew devoncarew requested a review from pq September 15, 2021 17:38
Copy link
Contributor

@pq pq left a comment

Choose a reason for hiding this comment

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

Nice!

Just some nits...

@jonasfj jonasfj merged commit c2dc871 into dart-archive:master Sep 17, 2021
@jonasfj jonasfj deleted the add-applicationConfigHome branch September 17, 2021 09:08
/// [XDG Base Directory Specification][1] on Linux and [File System Basics][2]
/// on Mac OS.
///
/// Throws if `%APPDATA%` or `$HOME` is undefined.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Throws if `%APPDATA%` or `$HOME` is undefined.
/// Throws if `%APPDATA%` or `$HOME` is needed but undefined.

/// * `$HOME/.config/<productName>` otherwise.
///
/// This aims follows best practices for each platform, honoring the
/// [XDG Base Directory Specification][1] on Linux and [File System Basics][2]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have documentation for the windows convention?

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 tried, I can't find any canonical source on MSDN... The closes I got was:
https://docs.microsoft.com/en-us/windows/deployment/usmt/usmt-recognized-environment-variables

But I have no idea what that page actually pertains to.. It looks like it says:

When using the XML files MigDocs.xml, MigApp.xml, and MigUser.xml, you can use environment variables to identify folders that may be different on different computers

So it pertains to people writing such XML files.. and not as general documentation of the Windows environment variables. Or at-least this is not clear to me 🙈

There is also references to Windows File System Namespace Usage Guidelines floating around, but again I can't find updated documentation from original source.

Who knows... maybe it's hidden behind an MSDN subscription? Or maybe it's just so widely known and there is so many search hits that finding the canonical documentation is very hard.

We could use wikipedia:

I just gave up 🤣


group('applicationConfigHome', () {
test('returns a string', () {
expect(applicationConfigHome('dart'), isA<String>());
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm - this is hard to test - but it seems we should be able to do better.

Could we at least confirm that the string ends with '${separator}dart'

Copy link
Contributor

@sigurdm sigurdm left a comment

Choose a reason for hiding this comment

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

LGTM - I wonder if we can write a better test though.

mosuem pushed a commit to dart-lang/tools that referenced this pull request Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

Adhere to XDG base directory spec
3 participants