-
-
Notifications
You must be signed in to change notification settings - Fork 248
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
Bookmarks improvements #changed #659
Conversation
* main: (50 commits) Update strings Version Bump to 3006 Fix button mask re-added credits and license files Remove archive scripts that update project version - we do that through fastlane now Version Bump to 3005 Update strings Changelog updates Change encoding to proper utf8 update project update es files Change localizable string Update strings Fix localizable strings diffing fixed unti test filename hopefully this fixes it Release the window when closed instead of nilling the document Don't hold nib objects in array for no reason xcode tweak temp: swizzle initFileURLWithPath: isDirectory ... # Conflicts: # Source/Controllers/MainViewControllers/ConnectionView/SPConnectionController.m # sequel-ace.xcodeproj/project.pbxproj
* main: Version Bump to 3007 Hotfix disable alerts for submit Fix keychain - get password on favorites save only if the filled in password is empty Clanup unused objects
* main: added DispatchQueue.main.async to each alert logging and extra check logging Maybe fix SPQueryController addHistory:forFileURL guard when taking substring of query PR feedback re query string fix SPCustomQuery performQueriesTask crash handle SPHistoryController historyControlClicked crash logging fixed SPFieldEditorController segmentControllerChanged crash fix numerous background thread crashes fix SPCopyTable rowsAsSqlInsertsOnlySelectedRows:skipAutoIncrementColumn crash fix-alias-completions some fixes re secure bookmark generation and logging # Conflicts: # Source/Controllers/DataExport/SPExportController.m # Source/Controllers/MainViewControllers/ConnectionView/SPConnectionController.m # Source/Controllers/Preferences/Panes/SPFilePreferencePane.m # Source/Controllers/Preferences/Panes/SPNetworkPreferencePane.m
* main: extra crashlytics logging for filehandles fix-csv-import-crash
* main: logging for missing key
* main: Update strings Fix versioning override Update CHANGELOG.md Update fastlane docu Changelog updates Increment build version Increment app patch version Revert "Increment app patch version" Increment app patch version Add new fastlane shortcuts, fix versioning Reformat custom query spaces Fix query commenting out Update templates Fix missing MainMenu xib for non-en localizations Fix time formatter
which calls _refreshBookmarks
* main: Update bug_report.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
Generally:
- you don't need any
public
at all - you should add
private
to everything that is not used outside of the class - try to avoid
!
:)
Source/Controllers/MainViewControllers/ConnectionView/SPConnectionController.m
Outdated
Show resolved
Hide resolved
* main: (37 commits) handle encoding popup title being nil guards around nil keys guard table Collation being NSNull fix-deriveQueryString-crash added logging for crash tests for safeObjectAtIndex NSArrayObjectAtIndex -> safeObjectAtIndex 5 NSArrayObjectAtIndex -> safeObjectAtIndex 4 NSArrayObjectAtIndex -> safeObjectAtIndex 3 NSArrayObjectAtIndex -> safeObjectAtIndex 2 NSArrayObjectAtIndex -> safeObjectAtIndex removed some logging Kaspik feedback Update Tabs close and plus buttons for light and dark mode Get rid of the PSMTabBar target, it's for nothing, just causes issues Fix crash Added CR logging to RegexKit's Exception and Error generation code. rm RegexKitLite from SequelAceTunnelAssistant swift version of stringByMatching:(NSString *)regex capture handle queryDbStructureWithUserInfo crash ... # Conflicts: # Source/Controllers/MainViewControllers/ConnectionView/SPConnectionController.m
* main: added bool property if error currently displayed
* main: Cleanup Fixed commenting for one or multiple lines
Ready to be opened for review @jamesstout ? |
Yes I think so now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Can't approve as it's my PR, so maybe do final review of your own code and approve the PR if it looks good @jamesstout :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
But, I would feel more comfortable including this in a 3.1.0 release and not including it in the current 3.0.2 bug fix release. I think this all looks great, but we do have the potential to introduce some new crashes and weird edge cases. So I would feel better if we got 3.0.2 out and then pulled this in and got some solid beta testing with this prior to release
* main: (31 commits) Don't reset animations on another animations Revert "Revert "Rename methods"" Fix crash Revert "Rename methods" Rename methods Cleanup animations Make behavior the same as Safari, don§t hide tabs Fix crashlytics script Update strings Fix fastlane docu Changelog updates Increment build version Fix checkout command Remove unnecessary command added guard match.location >= fieldMappingArray.count fixed incorrect array improvements to console Remove a few extraneous log messages Revert "Increment build version" Revert "Increment build version" ... # Conflicts: # Source/Controllers/MainViewControllers/ConnectionView/SPConnectionController.m
Going to give this all the once over again. |
Concerned, that on release, all bookmarks will be stale ... the user has to reselect them, we can't populate a list they have to be selected. |
That's kind of a big issue, right? Users might not even remember what they had selected. |
I created a popup ... I'll show you a screenshot .... |
I've added a migration step that .. amazingly, works |
* main: Window improvements Merge conflict fix Fix quick connect not clearing fields update strings after merge rename tmpStr to queryStr Copy tables between databases some array tests main thread errors # Conflicts: # Source/Controllers/MainViewControllers/ConnectionView/SPConnectionController.m # sequel-ace.xcodeproj/project.pbxproj
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes:
Closes following issues:
Tested:
Screenshots:
Additional notes: