diff --git a/CHANGELOG.md b/CHANGELOG.md index 104c37b..3c0a76c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed - Favicon support removed (#199) +- Bookmarks are stored using XML in the XBEL format, old bookmarks are transferred (#68) ### Fixed - Help text is now the same color as `regular_text` in the theme config - Non-ASCII (multibyte) characters can now be used as keybindings (#198, #200) +- Possible subscription update race condition on startup ## [1.8.0] - 2021-02-17 diff --git a/amfora.go b/amfora.go index b3f0359..db53fbc 100644 --- a/amfora.go +++ b/amfora.go @@ -4,6 +4,7 @@ import ( "fmt" "os" + "github.com/makeworld-the-better-one/amfora/bookmarks" "github.com/makeworld-the-better-one/amfora/client" "github.com/makeworld-the-better-one/amfora/config" "github.com/makeworld-the-better-one/amfora/display" @@ -44,13 +45,18 @@ func main() { fmt.Fprintf(os.Stderr, "Config error: %v\n", err) os.Exit(1) } + client.Init() + err = subscriptions.Init() if err != nil { fmt.Fprintf(os.Stderr, "subscriptions.json error: %v\n", err) os.Exit(1) } - - client.Init() + err = bookmarks.Init() + if err != nil { + fmt.Fprintf(os.Stderr, "bookmarks.xml error: %v\n", err) + os.Exit(1) + } // Initialize lower-level cview app if err = display.App.Init(); err != nil { diff --git a/bookmarks/bookmarks.go b/bookmarks/bookmarks.go index 194c357..126fd86 100644 --- a/bookmarks/bookmarks.go +++ b/bookmarks/bookmarks.go @@ -2,55 +2,88 @@ package bookmarks import ( "encoding/base32" + "encoding/xml" + "fmt" + "io/ioutil" + "os" "sort" "strings" "github.com/makeworld-the-better-one/amfora/config" ) -var bkmkStore = config.BkmkStore +func Init() error { + f, err := os.Open(config.BkmkPath) + if err == nil { + // File exists and could be opened -// bkmkKey returns the viper key for the given bookmark URL. -// Note that URLs are the keys, NOT the bookmark name. -func bkmkKey(url string) string { - // Keys are base32 encoded URLs to prevent any special chars like periods from being used - return "bookmarks." + base32.StdEncoding.EncodeToString([]byte(url)) -} + fi, err := f.Stat() + if err == nil && fi.Size() > 0 { + // File is not empty -func Set(url, name string) { - bkmkStore.Set(bkmkKey(url), name) - bkmkStore.WriteConfig() //nolint:errcheck -} - -// Get returns the NAME of the bookmark, given the URL. -// It also returns a bool indicating whether it exists. -func Get(url string) (string, bool) { - name := bkmkStore.GetString(bkmkKey(url)) - return name, name != "" -} - -func Remove(url string) { - // XXX: Viper can't actually delete keys, which means the bookmarks file might get clouded - // with non-entries over time. - bkmkStore.Set(bkmkKey(url), "") - bkmkStore.WriteConfig() //nolint:errcheck -} - -// All returns all the bookmarks in a map of URLs to names. -// It also returns a slice of map keys, sorted so that the map *values* -// are in alphabetical order, with case ignored. -func All() (map[string]string, []string) { - bkmks := make(map[string]string) - - bkmksMap, ok := bkmkStore.AllSettings()["bookmarks"].(map[string]interface{}) - if !ok { - // No bookmarks stored yet, return empty map - return bkmks, []string{} + xbelBytes, err := ioutil.ReadAll(f) + f.Close() + if err != nil { + return fmt.Errorf("read bookmarks.xml error: %w", err) + } + err = xml.Unmarshal(xbelBytes, &data) + if err != nil { + return fmt.Errorf("bookmarks.xml is corrupted: %w", err) + } + } + f.Close() + } else if !os.IsNotExist(err) { + // There's an error opening the file, but it's not bc is doesn't exist + return fmt.Errorf("open bookmarks.xml error: %w", err) } - inverted := make(map[string]string) // Holds inverted map, name->URL - names := make([]string, 0, len(bkmksMap)) // Holds bookmark names, for sorting - keys := make([]string, 0, len(bkmksMap)) // Final sorted keys (URLs), for returning at the end + if data.Bookmarks == nil { + data.Bookmarks = make([]*xbelBookmark, 0) + data.Version = xbelVersion + } + + if config.BkmkStore != nil { + // There's still bookmarks stored in the old format + // Add them and delete the file + + names, urls := oldBookmarks() + for i := range names { + data.Bookmarks = append(data.Bookmarks, &xbelBookmark{ + URL: urls[i], + Name: names[i], + }) + } + + err := writeXbel() + if err != nil { + return fmt.Errorf("error saving old bookmarks into new format: %w", err) + } + + err = os.Remove(config.OldBkmkPath) + if err != nil { + return fmt.Errorf( + "couldn't delete old bookmarks file (%s), you must delete it yourself to prevent duplicate bookmarks: %w", + config.OldBkmkPath, + err, + ) + } + config.BkmkStore = nil + } + + return nil +} + +// oldBookmarks returns a slice of names and a slice of URLs of the +// bookmarks in config.BkmkStore. +func oldBookmarks() ([]string, []string) { + bkmksMap, ok := config.BkmkStore.AllSettings()["bookmarks"].(map[string]interface{}) + if !ok { + // No bookmarks stored yet, return empty map + return []string{}, []string{} + } + + names := make([]string, 0, len(bkmksMap)) + urls := make([]string, 0, len(bkmksMap)) for b32Url, name := range bkmksMap { if n, ok := name.(string); n == "" || !ok { @@ -63,15 +96,89 @@ func All() (map[string]string, []string) { // This would only happen if a user messed around with the bookmarks file continue } - bkmks[string(url)] = name.(string) - inverted[name.(string)] = string(url) names = append(names, name.(string)) + urls = append(urls, string(url)) } - // Sort, then turn back into URL keys - sort.Strings(names) - for _, name := range names { - keys = append(keys, inverted[name]) + return names, urls +} + +func writeXbel() error { + xbelBytes, err := xml.MarshalIndent(&data, "", " ") + if err != nil { + return err } - return bkmks, keys + xbelBytes = append(xbelHeader, xbelBytes...) + err = ioutil.WriteFile(config.BkmkPath, xbelBytes, 0666) + if err != nil { + return err + } + return nil +} + +// Change the name of the bookmark at the provided URL. +func Change(url, name string) { + for _, bkmk := range data.Bookmarks { + if bkmk.URL == url { + bkmk.Name = name + writeXbel() //nolint:errcheck + return + } + } +} + +// Add will add a new bookmark. +func Add(url, name string) { + data.Bookmarks = append(data.Bookmarks, &xbelBookmark{ + URL: url, + Name: name, + }) + writeXbel() //nolint:errcheck +} + +// Get returns the NAME of the bookmark, given the URL. +// It also returns a bool indicating whether it exists. +func Get(url string) (string, bool) { + for _, bkmk := range data.Bookmarks { + if bkmk.URL == url { + return bkmk.Name, true + } + } + return "", false +} + +func Remove(url string) { + for i, bkmk := range data.Bookmarks { + if bkmk.URL == url { + data.Bookmarks[i] = data.Bookmarks[len(data.Bookmarks)-1] + data.Bookmarks = data.Bookmarks[:len(data.Bookmarks)-1] + writeXbel() //nolint:errcheck + return + } + } +} + +// All returns all the bookmarks in a map of URLs to names. +// It also returns a slice of map keys, sorted so that the map *values* +// are in alphabetical order, with case ignored. +func All() (map[string]string, []string) { + bkmksMap := make(map[string]string) + + inverted := make(map[string]string) // Holds inverted map, name->URL + names := make([]string, len(data.Bookmarks)) // Holds bookmark names, for sorting + keys := make([]string, len(data.Bookmarks)) // Final sorted keys (URLs), for returning at the end + + for i, bkmk := range data.Bookmarks { + bkmksMap[bkmk.URL] = bkmk.Name + inverted[bkmk.Name] = bkmk.URL + names[i] = bkmk.Name + } + + // Sort, then turn back into URL keys + sort.Strings(names) + for i, name := range names { + keys[i] = inverted[name] + } + + return bkmksMap, keys } diff --git a/bookmarks/xbel.go b/bookmarks/xbel.go new file mode 100644 index 0000000..bf1f132 --- /dev/null +++ b/bookmarks/xbel.go @@ -0,0 +1,43 @@ +package bookmarks + +// Structs and code for the XBEL XML bookmark format. +// https://github.com/makeworld-the-better-one/amfora/issues/68 + +import ( + "encoding/xml" +) + +var xbelHeader = []byte(xml.Header + ` +`) + +const xbelVersion = "1.1" + +type xbelBookmark struct { + XMLName xml.Name `xml:"bookmark"` + URL string `xml:"href,attr"` + Name string `xml:"title"` +} + +// xbelFolder is unused as folders aren't supported by the UI yet. +// Follow #56 for details. +// https://github.com/makeworld-the-better-one/amfora/issues/56 +type xbelFolder struct { + XMLName xml.Name `xml:"folder"` + Version string `xml:"version,attr"` + Folded string `xml:"folded,attr"` // Idk if this will be used or not + Name string `xml:"title"` + Bookmarks []*xbelBookmark `xml:"bookmark"` + Folders []*xbelFolder `xml:"folder"` +} + +type xbel struct { + XMLName xml.Name `xml:"xbel"` + Version string `xml:"version,attr"` + Bookmarks []*xbelBookmark `xml:"bookmark"` + // Later: Folders []*xbelFolder +} + +// Instance of xbel - loaded from bookmarks file +var data xbel diff --git a/config/config.go b/config/config.go index b0d8fed..42b391f 100644 --- a/config/config.go +++ b/config/config.go @@ -32,10 +32,10 @@ var tofuDBDir string var tofuDBPath string // Bookmarks - -var BkmkStore = viper.New() +var BkmkStore = viper.New() // TOML API for old bookmarks file var bkmkDir string -var bkmkPath string +var OldBkmkPath string // Old bookmarks file that used TOML format +var BkmkPath string // New XBEL (XML) bookmarks file, see #68 var DownloadsDir string var TempDownloadsDir string @@ -111,7 +111,8 @@ func Init() error { // XDG data dir on POSIX systems bkmkDir = filepath.Join(basedir.DataHome, "amfora") } - bkmkPath = filepath.Join(bkmkDir, "bookmarks.toml") + OldBkmkPath = filepath.Join(bkmkDir, "bookmarks.toml") + BkmkPath = filepath.Join(bkmkDir, "bookmarks.xml") // Feeds dir and path if runtime.GOOS == "windows" { @@ -160,10 +161,8 @@ func Init() error { if err != nil { return err } - f, err = os.OpenFile(bkmkPath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) - if err == nil { - f.Close() - } + // OldBkmkPath isn't created because it shouldn't be there anyway + // Feeds err = os.MkdirAll(subscriptionDir, 0755) if err != nil { @@ -179,16 +178,12 @@ func Init() error { return err } - BkmkStore.SetConfigFile(bkmkPath) + BkmkStore.SetConfigFile(OldBkmkPath) BkmkStore.SetConfigType("toml") err = BkmkStore.ReadInConfig() if err != nil { - return err - } - BkmkStore.Set("DO NOT TOUCH", true) - err = BkmkStore.WriteConfig() - if err != nil { - return err + // File doesn't exist, so remove the viper + BkmkStore = nil } // Setup main config diff --git a/display/bookmarks.go b/display/bookmarks.go index 083a902..40c8d73 100644 --- a/display/bookmarks.go +++ b/display/bookmarks.go @@ -15,9 +15,18 @@ import ( // For adding and removing bookmarks, basically a clone of the input modal. var bkmkModal = cview.NewModal() +type bkmkAction int + +const ( + add bkmkAction = iota + change + cancel + remove +) + // bkmkCh is for the user action -var bkmkCh = make(chan int) // 1, 0, -1 for add/update, cancel, and remove -var bkmkModalText string // The current text of the input field in the modal +var bkmkCh = make(chan bkmkAction) +var bkmkModalText string // The current text of the input field in the modal func bkmkInit() { panels.AddPanel("bkmk", bkmkModal, false, false) @@ -60,15 +69,15 @@ func bkmkInit() { m.SetDoneFunc(func(buttonIndex int, buttonLabel string) { switch buttonLabel { case "Add": - bkmkCh <- 1 + bkmkCh <- add case "Change": - bkmkCh <- 1 + bkmkCh <- change case "Remove": - bkmkCh <- -1 + bkmkCh <- remove case "Cancel": - bkmkCh <- 0 + bkmkCh <- cancel case "": - bkmkCh <- 0 + bkmkCh <- cancel } }) } @@ -76,9 +85,8 @@ func bkmkInit() { // Bkmk displays the "Add a bookmark" modal. // It accepts the default value for the bookmark name that will be displayed, but can be changed by the user. // It also accepts a bool indicating whether this page already has a bookmark. -// It returns the bookmark name and the bookmark action: -// 1, 0, -1 for add/update, cancel, and remove -func openBkmkModal(name string, exists bool) (string, int) { +// It returns the bookmark name and the bookmark action. +func openBkmkModal(name string, exists bool) (string, bkmkAction) { // Basically a copy of Input() // Reset buttons before input field, to make sure the input is in focus @@ -152,11 +160,12 @@ func addBookmark() { // Open a bookmark modal with the current name of the bookmark, if it exists newName, action := openBkmkModal(name, exists) switch action { - case 1: - // Add/change the bookmark - bookmarks.Set(p.URL, newName) - case -1: + case add: + bookmarks.Add(p.URL, newName) + case change: + bookmarks.Change(p.URL, newName) + case remove: bookmarks.Remove(p.URL) } - // Other case is action = 0, meaning "Cancel", so nothing needs to happen + // Other case is action == cancel, so nothing needs to happen }