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

Writefreely import #2

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Writefreely import #2

wants to merge 17 commits into from

Conversation

joyeusenoelle
Copy link

Adapts the existing writeas import tool to work with any WriteFreely instance.

This tool will prompt for login information if the instance isn't stored in the included INI file. It does not currently support the config being developed elsewhere; I have plans to include that in a later revision.

It also detects blog-name collisions and adds a "friendly" 4-character string to the end of each blog's name if a blog with that name already exists in the database.

@joyeusenoelle joyeusenoelle requested a review from thebaer June 21, 2019 13:48
@joyeusenoelle
Copy link
Author

joyeusenoelle commented Jun 21, 2019

wp-import no longer simply trusts the user to feed it the right file, and now verifies that the file it's importing is a WordPress file, by checking the file extension and searching the first 200 bytes for the word "WordPress" (this appears in the comment block at the top of all WP exports).

I've also generalized the main function so that it takes a (new) ImportedBlogs struct (defined here) instead of a WXR struct, and written the function to convert the WXR struct to the new ImportedBlog format. This will make generalizing the tool to import other formats (zip, JSON, etc.) easier.

Copy link
Member

@thebaer thebaer left a comment

Choose a reason for hiding this comment

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

Thanks @joyeusenoelle, this is great progress. I haven't run this yet, but noticed a few things we should address after reading through the code.

main.go Outdated
// wp-import -i instance filename
// wp-import -i instance -f filename

func parseArgs(args []string) map[string]string {
Copy link
Member

Choose a reason for hiding this comment

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

I think we can get rid of this func and make parsing less error-prone by using the built-in flag package. Here's a good example of how that can be done: https://github.com/writeas/wf-migrate/blob/master/cmd/wfimport/main.go

Copy link
Author

Choose a reason for hiding this comment

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

After seeing your use of flag there I was planning on switching to it. :)

main.go Outdated
errQuit("usage: wp-import filename.xml")
// Print the usage spec to the terminal and exit cleanly
func printUsage(help bool) {
usage := "usage: wp-import [-h|--help] [-i instance] [-f] filename.xml"
Copy link
Member

Choose a reason for hiding this comment

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

As a bonus of using the flag package (as mentioned in another comment), we'll get all this output for free. So we probably won't need this func either.

main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I am not able to test but overall looks good.

@joyeusenoelle
Copy link
Author

I've modified the code to use the flag package, and removed the now-extraneous parseArgs and printUsage functions.

I've also added the capacity to do a dry run, so users can verify that their file parses correctly before they upload everything to the remote server. (Also useful for testing changes that don't affect the upload process. ;)

Final commit of the day 6/21. Code doesn't currently work, but I see how to get it there.
Copy link
Author

@joyeusenoelle joyeusenoelle left a comment

Choose a reason for hiding this comment

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

Ignore comment on 67-68 - I changed my mind. We're exporting ParseWPFile so it should handle the WXR stuff and not rely on the importing script to do that. ParseWPFile now takes []byte as an argument.

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