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

Ignore conflicts between hidden files #2148

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JonathanFeenstra
Copy link
Contributor

Conflicts between hidden files are now no longer considered conflicts and will no longer show up in the mod conflict flags, the (advanced) conflicts tab, or the data tab if "Show conflicts only" is checked. The "Unhide" context menu action is also removed from the conflicts tab, since it's no longer used if hidden files no longer show up there.

Regarding the data tab filters, I considered force-unchecking and disabling the "Show hidden files" checkbox as soon as "Show conflicts only" filter is checked as an alternative implementation, but I think that will only lead to more complex code and worse UX since the user would have to re-check it again afterwards.

This was listed as a suggestion in #464.

Comment on lines +973 to +978
if (showConflictsOnly() &&
((file.getAlternatives().size() == 0) ||
fs::path(file.getName())
.extension()
.compare(ModInfo::s_HiddenExt.toStdWString()) == 0)) {
// only conflicts should be shown, but this file is hidden or not conflicted
Copy link
Member

Choose a reason for hiding this comment

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

This should be a hotpath, I don't know how fast the fs:path conversion to compare the extension is. Might be worth profiling this under a large setup. I know we got massive wins by using a faster string comparison in the past

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think file.getName().ends_with(ModInfo::s_HiddenExt.toStdWString()) would suffice? That's what I used here. I only used the fs::path conversion here to be consistent with similar existing code that I used as a reference.

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.

2 participants