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

Fix "Go to parent folder" in EditorFileDialog #80821

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

Alex2782
Copy link
Contributor

Fixes: #80750

I am not sure if '/' character may be removed in the function get_base_dir() as well. (190 results in 80 files)

@Alex2782 Alex2782 force-pushed the bugfix_go_up_#80750 branch from fe836c2 to 802a96f Compare August 20, 2023 15:37
@AThousandShips AThousandShips changed the title Fix go to parent folder Fix go to parent folder in EditorFileDialog Sep 3, 2023
@AThousandShips AThousandShips requested a review from a team September 3, 2023 18:23
@KoBeWi
Copy link
Member

KoBeWi commented Sep 5, 2023

I am not sure if '/' character may be removed in the function get_base_dir() as well. (190 results in 80 files)

I remember we have workaround code for the / issue in various places. Might be indeed worth it to modify get_base_dir() itself (but then we should do a cleanup pass to remove the workarounds).

@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Sep 5, 2023
@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 5, 2023

no problems

Screen-2023-09-06-004217.mp4

@Alex2782 Alex2782 force-pushed the bugfix_go_up_#80750 branch from 802a96f to b030209 Compare September 5, 2023 22:47
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

I can't reproduce the issue myself, but the fix looks fine.

@Alex2782 Alex2782 force-pushed the bugfix_go_up_#80750 branch from b030209 to 40046bc Compare September 5, 2023 23:01
@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 5, 2023

maybe in one line? Cleaner?

image

or better?
(I personally try to keep the parameters that are passed to the functions short.)

void EditorFileDialog::_go_up() {
	String up_dir = get_current_dir().trim_suffix("/").get_base_dir();
	dir_access->change_dir(up_dir);
	
	update_file_list();
	update_dir();
	_push_history();
}

@akien-mga akien-mga added the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Sep 7, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good.

@akien-mga akien-mga merged commit 88269cf into godotengine:master Sep 7, 2023
@akien-mga
Copy link
Member

Thanks!

@Alex2782 Alex2782 deleted the bugfix_go_up_#80750 branch September 7, 2023 12:52
@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 24, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.3.

@YuriSizov YuriSizov changed the title Fix go to parent folder in EditorFileDialog Fix "Go to parent folder" in EditorFileDialog Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FileDialog considers /path/to/dir/'s parent directory to be path/to/dir
5 participants