Add Naive OPML Import Support#165
Conversation
3cac561 to
24e4024
Compare
guyfedwards
left a comment
There was a problem hiding this comment.
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
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) |
There was a problem hiding this comment.
Update error message to be config.ImportFeeds
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) |
There was a problem hiding this comment.
Ah yeah, that seems to have been caused by the auto formatter.
internal/config/config.go
Outdated
| } | ||
|
|
||
| err = os.WriteFile(c.ConfigPath, []byte(str), 0655) | ||
| err = os.WriteFile(c.ConfigPath, []byte(str), 0o655) |
There was a problem hiding this comment.
Ah yeah, that seems to have been caused by the auto formatter.
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 != "" { |
There was a problem hiding this comment.
For a more complex conditional, split it into separate lines:
URL, err := url.Parse(source);
if err == nil && URL.Host != "" && URL.Scheme != "" {|
Thanks @GV14982 🎉 |
|
@guyfedwards you cool to add the |
|
@GV14982 of course, should be done but let me know if you need it added anywhere else. Thanks for the contribution |
Closes #123