Skip to content

Add Naive OPML Import Support - #165

Merged
guyfedwards merged 5 commits into
guyfedwards:masterfrom
GV14982:feat/ompl-import
Oct 12, 2025
Merged

Add Naive OPML Import Support#165
guyfedwards merged 5 commits into
guyfedwards:masterfrom
GV14982:feat/ompl-import

Conversation

@GV14982

@GV14982 GV14982 commented Oct 11, 2025

Copy link
Copy Markdown
Contributor
  • feat: add basic command structure and some of initial plumbing
  • feat: initial implementation for OPML feed imports
  • docs: update README with new import command

Closes #123

@guyfedwards guyfedwards left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Overall looks great, thanks @GV14982. Few nitpicks and could you add a test for the opml parsing? I see you added a fixture but no test yet. Thanks

Comment thread README.md
Comment thread internal/config/config.go Outdated
func (c *Config) ImportFeeds() ([]Feed, error) {
err := c.Load()
if err != nil {
return nil, fmt.Errorf("config.AddFeed: %w", err)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Update error message to be config.ImportFeeds

Comment thread internal/config/config.go Outdated

// if not, create directory. noop if directory exists
err = os.MkdirAll(c.ConfigDir, 0755)
err = os.MkdirAll(c.ConfigDir, 0o755)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

unrelated change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah, that seems to have been caused by the auto formatter.

Comment thread internal/config/config.go Outdated
}

err = os.WriteFile(c.ConfigPath, []byte(str), 0655)
err = os.WriteFile(c.ConfigPath, []byte(str), 0o655)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

unrelated change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah, that seems to have been caused by the auto formatter.

Comment thread internal/commands/commands.go Outdated

func (c Commands) ImportFeeds(source string) error {
var opmlText string
if URL, err := url.Parse(source); err == nil && URL.Host != "" && URL.Scheme != "" {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

For a more complex conditional, split it into separate lines:

URL, err := url.Parse(source);
if err == nil && URL.Host != "" && URL.Scheme != "" {

@GV14982
GV14982 requested a review from guyfedwards October 12, 2025 03:28
@guyfedwards
guyfedwards merged commit a6b7a03 into guyfedwards:master Oct 12, 2025
2 checks passed
@guyfedwards

Copy link
Copy Markdown
Owner

Thanks @GV14982 🎉

@GV14982

GV14982 commented Oct 12, 2025

Copy link
Copy Markdown
Contributor Author

@guyfedwards you cool to add the hacktoberfest-accepted label to this 🙃 I'm doing a Hacktoberfest celebration with some friends and I gotta make sure to finish my 6 contributions 🤣

@guyfedwards

Copy link
Copy Markdown
Owner

@GV14982 of course, should be done but let me know if you need it added anywhere else. Thanks for the contribution

@guyfedwards

Copy link
Copy Markdown
Owner

released in v2.18.0, thanks @GV14982

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Import OPML

2 participants