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: creates file/directory in active path #1416

Conversation

EstebanBorai
Copy link
Member

@EstebanBorai EstebanBorai commented Sep 3, 2023

Description

Uses EditorManager to retrieve active directory and create Files/Directories in the
current active directory.

Related Issues

I found the todos in the source code while reading.
I wanted to address the related issue but didn't found one.

Here is the TODO comment: https://github.com/CodeEditApp/CodeEdit/blob/main/CodeEdit/Features/NavigatorSidebar/ProjectNavigator/ProjectNavigatorToolbarBottom.swift#L59

  • #ISSUE_NUMBER

N/A

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • [-] The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

Screen.Recording.2023-09-12.at.8.05.30.PM.mov

@matthijseikelenboom
Copy link
Contributor

The TODO comment is in the ProjectNavigator. You're accessing the TabBar to get the required data. While I believe that this does work, but it's not the correct way. We shouldn't want to mix those domains when not necessary. The ProjectNavigator should have a current selected file as well, so that should be accessed. As the ProjectNavigatorBottomBar is part of thé ProjectNavigator.

@EstebanBorai
Copy link
Member Author

The TODO comment is in the ProjectNavigator. You're accessing the TabBar to get the required data. While I believe that this does work, but it's not the correct way. We shouldn't want to mix those domains when not necessary. The ProjectNavigator should have a current selected file as well, so that should be accessed. As the ProjectNavigatorBottomBar is part of thé ProjectNavigator.

Makes sense! Will review!

@thecoolwinter
Copy link
Collaborator

Addressing the comment left in Discord: This is very close, instead of using the workspace document, use the editor manager directly via the following in ProjectNavigatorToolbarBottom:

@EnvironmentObject var editorManager: EditorManager

You can use the editor manager to get the active editor pane, and then the selected tab from that. See the EditorManager.activeTabGroup, and Editor.selectedTab properties respectively.

That should get rid of the need to add methods to WorkspaceDocument and addresses the domain concern @matthijseikelenboom had.

@EstebanBorai
Copy link
Member Author

Addressing the comment left in Discord: This is very close, instead of using the workspace document, use the editor manager directly via the following in ProjectNavigatorToolbarBottom:

@EnvironmentObject var editorManager: EditorManager

You can use the editor manager to get the active editor pane, and then the selected tab from that. See the EditorManager.activeTabGroup, and Editor.selectedTab properties respectively.

That should get rid of the need to add methods to WorkspaceDocument and addresses the domain concern @matthijseikelenboom had.

Great! Thanks so much!

@EstebanBorai EstebanBorai force-pushed the feat/create-file-and-dir-in-active-path branch from babe0d1 to 1d83e83 Compare September 12, 2023 23:08
@EstebanBorai
Copy link
Member Author

Hi @thecoolwinter I just applied suggestions! Also added a recording for demo. Repo in demo is public available here: https://github.com/whizzes/playa

thecoolwinter
thecoolwinter previously approved these changes Sep 12, 2023
Copy link
Collaborator

@thecoolwinter thecoolwinter left a comment

Choose a reason for hiding this comment

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

Awesome! Looks good to me

@austincondiff
Copy link
Collaborator

austincondiff commented Sep 13, 2023

@EstebanBorai Could we highlight the text after the untitled file/folder is created so the user can simply type their desired name for it?

Copy link
Collaborator

@austincondiff austincondiff left a comment

Choose a reason for hiding this comment

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

LGTM!

@thecoolwinter thecoolwinter enabled auto-merge (squash) September 19, 2023 01:48
@thecoolwinter
Copy link
Collaborator

@matthijseikelenboom can we get a review here

@thecoolwinter thecoolwinter merged commit 0f0edd7 into CodeEditApp:main Sep 19, 2023
@austincondiff
Copy link
Collaborator

@allcontributors add @EstebanBorai for code

@allcontributors
Copy link
Contributor

@austincondiff

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

@thecoolwinter thecoolwinter added the enhancement New feature or request label Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants