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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Fix default dir for git repo init #1627

Merged
merged 1 commit into from Aug 3, 2018

Conversation

3 participants
@Wittiest
Contributor

Wittiest commented Aug 3, 2018

Description of the Change

The linked issue details the unexpected default directory when using the atom sidebar to initialize a git repository. This PR ensures that the default directory is the working directory, as would be expected.

Before this change, the conditional on line 35 of init-dialog.js would always set the default directory to the home directory.

Alternate Designs

An in-depth look at why initDialogPath is null here could possibly provide a more elegant solution; however, this is concise enough to fix the issue without needing consideration for negative fallouts.

Benefits

Users initializing a git repository with the sidebar will now have the default directory begin being set to the current working directory. This is what most users would expect, rather than it defaulting to the home directory.

Possible Drawbacks

None

Applicable Issues

#1237

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Aug 3, 2018

Coverage Status

Coverage decreased (-0.1%) to 79.562% when pulling 7109147 on Wittiest:master into a749da1 on atom:master.

coveralls commented Aug 3, 2018

Coverage Status

Coverage decreased (-0.1%) to 79.562% when pulling 7109147 on Wittiest:master into a749da1 on atom:master.

@smashwilson

This comment has been minimized.

Show comment
Hide comment
@smashwilson

smashwilson Aug 3, 2018

Member

Ah, excellent. Thanks, this had been bugging me 馃槃

An in-depth look at why initDialogPath is null here could possibly provide a more elegant solution; [..]

Here's the section that's supposed to be populating the path argument:

initializeRepo(event) {
event.preventDefault();
let initPath = null;
const activeEditor = this.props.workspace.getActiveTextEditor();
if (activeEditor) {
const [projectPath] = this.props.project.relativizePath(activeEditor.getPath());
if (projectPath) {
initPath = projectPath;
}
}
this.props.initializeRepo(initPath);
}

But this will only work if you have an open TextEditor focused in your workspace center, which is not always the case. We could likely take that from this.props.repository.getWorkingDirectoryPath() instead... ?

[..] however, this is concise enough to fix the issue without needing consideration for negative fallouts.

I agree. :shipit: 馃槈

Member

smashwilson commented Aug 3, 2018

Ah, excellent. Thanks, this had been bugging me 馃槃

An in-depth look at why initDialogPath is null here could possibly provide a more elegant solution; [..]

Here's the section that's supposed to be populating the path argument:

initializeRepo(event) {
event.preventDefault();
let initPath = null;
const activeEditor = this.props.workspace.getActiveTextEditor();
if (activeEditor) {
const [projectPath] = this.props.project.relativizePath(activeEditor.getPath());
if (projectPath) {
initPath = projectPath;
}
}
this.props.initializeRepo(initPath);
}

But this will only work if you have an open TextEditor focused in your workspace center, which is not always the case. We could likely take that from this.props.repository.getWorkingDirectoryPath() instead... ?

[..] however, this is concise enough to fix the issue without needing consideration for negative fallouts.

I agree. :shipit: 馃槈

@smashwilson smashwilson merged commit 8ffa076 into atom:master Aug 3, 2018

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.1%) to 79.562%
Details

@smashwilson smashwilson added this to In Progress 馃敡 in Stability Sprint : 23 July - 3 August 2018 : v0.19.0 via automation Aug 13, 2018

@smashwilson smashwilson moved this from In Progress 馃敡 to Merged 鈽戯笍 in Stability Sprint : 23 July - 3 August 2018 : v0.19.0 Aug 13, 2018

This was referenced Aug 13, 2018

smashwilson added a commit that referenced this pull request Aug 14, 2018

Merge pull request #1627 from Wittiest/master
馃悰 Fix default dir for git repo init
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment