Skip to content

Convert URL-related parts of Blog.m to Swift#25276

Merged
kean merged 1 commit intotrunkfrom
task/blog-swift-rewrite
Feb 20, 2026
Merged

Convert URL-related parts of Blog.m to Swift#25276
kean merged 1 commit intotrunkfrom
task/blog-swift-rewrite

Conversation

@kean
Copy link
Contributor

@kean kean commented Feb 20, 2026

This PR affects displayURL, loginURL, etc. In addition to a mechanical rewrite I changed some of these to be more idiomatic Swift: naming, URL?, etc.

@dangermattic
Copy link
Collaborator

1 Error
🚫 PR requires at least one label.
2 Warnings
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

}
}

// Used as a key to store passwords, if you change the algorithm, logins will break
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment seems obsolete. I found where the app uses self.xmlrpc, but nothing else.

/// Builds an admin URL by appending the given path to the admin base URL.
public func makeAdminURL(path: String) -> URL? {
var base = getOptionString(name: "admin_url") ?? url(withPath: "wp-admin/") ?? ""
if !base.hasSuffix("/") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There may be a better way to implement some of these, but I decided to keep direct ObjC to Swift conversion for now to reduce risk of breaking something.


if blog.isAtomic {
authenticationType = blog.isPrivate() ? .privateAtomic(blogID: dotComID) : .atomic(loginURL: blog.loginUrl())
authenticationType = blog.isPrivate() ? .privateAtomic(blogID: dotComID) : .atomic(loginURL: blog.loginURL?.absoluteString ?? "")
Copy link
Contributor Author

@kean kean Feb 20, 2026

Choose a reason for hiding this comment

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

I kept the existing login in tact and manually tested it:

Screen.Recording.2026-02-20.at.9.15.55.AM.mov

@kean kean force-pushed the task/blog-swift-rewrite branch from 0ee925b to 3d2ee7a Compare February 20, 2026 14:35
@sonarqubecloud
Copy link

@wpmobilebot
Copy link
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number31023
VersionPR #25276
Bundle IDorg.wordpress.alpha
Commit3d2ee7a
Installation URL6cm7gsqhr79eo
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number31023
VersionPR #25276
Bundle IDcom.jetpack.alpha
Commit3d2ee7a
Installation URL0u1hm8733kg58
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.


/// User-facing display URL with protocol and trailing slash stripped, IDN decoded.
@objc public var displayURL: String? {
guard let url else { return nil }
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't imagine a situation where the site URL should be nil?

But I guess that's another PR

@kean kean added this pull request to the merge queue Feb 20, 2026
Merged via the queue into trunk with commit 8a1d52c Feb 20, 2026
25 of 32 checks passed
@kean kean deleted the task/blog-swift-rewrite branch February 20, 2026 18:23
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.

4 participants