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

Contributions for 2ndalpha/gasmask#220 #1

Merged
merged 9 commits into from
Mar 27, 2023

Conversation

oliveratgithub
Copy link

@oliveratgithub oliveratgithub commented Mar 12, 2023

Contributions towards the PR 2ndalpha#220

Fixed some deprecation warnings and merged PRs

Bumped Sparkle and Gas Mask versions.

rstad and others added 9 commits January 23, 2019 09:52
As mentioned in Issues 2ndalpha#160 and 2ndalpha#140, the list of hosts is not
alphabetized in newer versions of macOS. This change is a feeble
attempt by me to sort the `hostsFiles` array as it's populated.

It seems to have built successfully on my machine but I'm
extremely new to the world of Obj-C development so it would not
surprise me in the least if this does something in a bad or
easily improved way.

That is, I added a way to Sort (`NSSortDescriptor`) and call it
immediately after every time we `addObject` to `hostsFiles` during
`loadFiles` in LocalHostsController.m.

Doing this after every addition seems like it might be a poor
choice from a performance perspective.  Moreover, it may be
wholly unnecessary when used in some environments where this issue
doesn't present.  I went with it anyway because I didn't know how
else to approach this, nor how to conditionally use this logic
only when I know it to be necessary.
Move host file storage to "~/Library/Application Support/Gas Mask"
Sort Local hosts when loading
Copy link
Owner

@sweetppro sweetppro 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 thx!
apologies for taking so long.

@sweetppro sweetppro merged commit 61dcc29 into sweetppro:master Mar 27, 2023
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.

5 participants