Fix bug migrating config, add test

.github/FUNDING.yml

@@ -0,0 +1 @@
+github: USERNAME

.vscode/settings.json

@@ -0,0 +1,9 @@
+{
+    "cSpell.words": [
+        "Cleanup",
+        "tmpl",
+        "vars",
+        "gropple"
+    ],
+    "cSpell.language": "en-GB"
+}

CHANGELOG.md

@@ -4,6 +4,19 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+## [v0.5.5] - 2022-04-09
+
+- Fix a bug which would erase configuration when migrating from v1 to v2 config
+
+## [v0.5.4] - 2022-04-07
+
+- Check the chosen command exists when configuring a profile
+- Improve documentation
+- Add a stop button in the popup to abort a download (Linux/Mac only)
+- Move included JS to local app instead of accessing from a CDN
+- Make the simultaneous download limit apply to each unique domain
+- Support "portable" mode, reading gropple.yml from the current directory, if present
+
 ## [v0.5.3] - 2021-11-21
 
 - Add config option to limit number of simultaneous downloads

README.md

@@ -1,6 +1,6 @@
 # gropple
 
-A frontend to youtube-dl (and forks) to download videos with a single click, straight from your web browser.
+A frontend to youtube-dl (or compatible forks, like yt-dlp) to download videos with a single click, straight from your web browser.
 
 ![Screencast](/screencast.gif)
 
@@ -75,6 +75,13 @@ While gropple will use your `PATH` to find the executable, you can also specify
 instead. Note that any tools that the downloader calls itself (for instance, ffmpeg) will 
 probably need to be available on your path.
 
+## Portable mode
+
+If you'd like to use gropple from a USB stick or similar, copy the config file from
+it's default location (shown when you start gropple) to the same location as the binary, and rename it to `gropple.yml`. 
+
+If that file is present in the same directory as the binary, it will be used instead.
+
 ## Problems
 
 Most download problems are probably diagnosable via the log - check in the popup window and scroll

config/config.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"os/exec"
 	"strings"
 
 	"gopkg.in/yaml.v2"
@@ -36,13 +37,19 @@ type Config struct {
 	DownloadProfiles []DownloadProfile `yaml:"profiles" json:"profiles"`
 }
 
-func TestConfig() *Config {
+// ConfigService is a struct to handle configuration requests, allowing for the
-	config := DefaultConfig()
+// location that config files are loaded to be customised.
-	config.DownloadProfiles = []DownloadProfile{{Name: "test profile", Command: "sleep", Args: []string{"5"}}}
+type ConfigService struct {
-	return config
+	Config     *Config
+	ConfigPath string
 }
 
-func DefaultConfig() *Config {
+func (cs *ConfigService) LoadTestConfig() {
+	cs.LoadDefaultConfig()
+	cs.Config.DownloadProfiles = []DownloadProfile{{Name: "test profile", Command: "sleep", Args: []string{"5"}}}
+}
+
+func (cs *ConfigService) LoadDefaultConfig() {
 	defaultConfig := Config{}
 	stdProfile := DownloadProfile{Name: "standard video", Command: "youtube-dl", Args: []string{
 		"--newline",
@@ -71,7 +78,9 @@ func DefaultConfig() *Config {
 
 	defaultConfig.ConfigVersion = 2
 
-	return &defaultConfig
+	cs.Config = &defaultConfig
+
+	return
 }
 
 func (c *Config) ProfileCalled(name string) *DownloadProfile {
@@ -147,13 +156,28 @@ func (c *Config) UpdateFromJSON(j []byte) error {
 			}
 		}
 
+		// check the command exists
+		_, err := exec.LookPath(newConfig.DownloadProfiles[i].Command)
+		if err != nil {
+			return fmt.Errorf("Could not find %s on the path", newConfig.DownloadProfiles[i].Command)
+		}
 	}
 
 	*c = newConfig
 	return nil
 }
 
-func configPath() string {
+// DetermineConfigDir determines where the config is (or should be) stored.
+func (cs *ConfigService) DetermineConfigDir() {
+	// check current directory first, for a file called gropple.yml
+	_, err := os.Stat("gropple.yml")
+	if err == nil {
+		// exists in current directory, use that.
+		cs.ConfigPath = "gropple.yml"
+		return
+	}
+
+	// otherwise fall back to using the UserConfigDir
 	dir, err := os.UserConfigDir()
 	if err != nil {
 		log.Fatalf("cannot find a directory to store config: %v", err)
@@ -171,36 +195,40 @@ func configPath() string {
 	}
 
 	fullFilename := fullPath + string(os.PathSeparator) + "config.yml"
-	return fullFilename
+	cs.ConfigPath = fullFilename
 }
 
-func ConfigFileExists() bool {
+// ConfigFileExists checks if the config file already exists, and also checks
-	info, err := os.Stat(configPath())
+// if there is an error accessing it
+func (cs *ConfigService) ConfigFileExists() (bool, error) {
+	path := cs.ConfigPath
+	info, err := os.Stat(path)
 	if os.IsNotExist(err) {
-		return false
+		return false, nil
 	}
 	if err != nil {
-		log.Fatal(err)
+		return false, fmt.Errorf("could not check if '%s' exists: %s", path, err)
 	}
 	if info.Size() == 0 {
-		log.Print("config file is 0 bytes?")
+		return false, errors.New("config file is 0 bytes")
-		return false
 	}
-	return true
+	return true, nil
 }
 
-func LoadConfig() (*Config, error) {
+// LoadConfig loads the configuration from disk, migrating and updating it to the
-	path := configPath()
+// latest version if needed.
+func (cs *ConfigService) LoadConfig() error {
+	path := cs.ConfigPath
 	b, err := os.ReadFile(path)
 	if err != nil {
-		log.Printf("Could not read config '%s': %v", path, err)
+		return fmt.Errorf("Could not read config '%s': %v", path, err)
-		return nil, err
 	}
 	c := Config{}
+	cs.Config = &c
+
 	err = yaml.Unmarshal(b, &c)
 	if err != nil {
-		log.Printf("Could not parse YAML config '%s': %v", path, err)
+		return fmt.Errorf("Could not parse YAML config '%s': %v", path, err)
-		return nil, err
 	}
 
 	// do migrations
@@ -214,30 +242,29 @@ func LoadConfig() (*Config, error) {
 
 	if configMigrated {
 		log.Print("Writing new config after version migration")
-		c.WriteConfig()
+		cs.WriteConfig()
 	}
 
-	return &c, nil
+	return nil
 }
 
-func (c *Config) WriteConfig() {
+// WriteConfig writes the in-memory config to disk.
-	s, err := yaml.Marshal(c)
+func (cs *ConfigService) WriteConfig() {
+	s, err := yaml.Marshal(cs.Config)
 	if err != nil {
 		panic(err)
 	}
 
-	path := configPath()
+	path := cs.ConfigPath
 	file, err := os.Create(
 		path,
 	)
 
 	if err != nil {
-		log.Fatalf("Could not open config file")
+		log.Fatalf("Could not open config file %s: %s", path, err)
 	}
 	defer file.Close()
 
 	file.Write(s)
 	file.Close()
-
-	log.Printf("Wrote configuration out to %s", path)
 }

config/config_test.go

@@ -0,0 +1,57 @@
+package config
+
+import (
+	"os"
+	"testing"
+)
+
+func TestMigrationV1toV2(t *testing.T) {
+	v2Config := `config_version: 1
+server:
+  port: 6123
+  address: http://localhost:6123
+  download_path: ./
+ui:
+  popup_width: 500
+  popup_height: 500
+profiles:
+- name: standard video
+  command: youtube-dl
+  args:
+  - --newline
+  - --write-info-json
+  - -f
+  - bestvideo[ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]/best
+- name: standard mp3
+  command: youtube-dl
+  args:
+  - --newline
+  - --write-info-json
+  - --extract-audio
+  - --audio-format
+  - mp3
+`
+	cs := configServiceFromString(v2Config)
+	err := cs.LoadConfig()
+	if err != nil {
+		t.Errorf("got error when loading config: %s", err)
+	}
+	if cs.Config.ConfigVersion != 2 {
+		t.Errorf("did not migrate version (it is '%d')", cs.Config.ConfigVersion)
+	}
+	if cs.Config.Server.MaximumActiveDownloads != 2 {
+		t.Error("did not add MaximumActiveDownloads")
+	}
+	t.Log(cs.ConfigPath)
+}
+
+func configServiceFromString(configString string) *ConfigService {
+	tmpFile, _ := os.CreateTemp("", "gropple_test_*.yml")
+	tmpFile.Write([]byte(configString))
+	tmpFile.Close()
+	cs := ConfigService{
+		Config:     &Config{},
+		ConfigPath: tmpFile.Name(),
+	}
+	return &cs
+}

download/download.go

@@ -3,11 +3,15 @@ package download
 import (
 	"fmt"
 	"io"
+	"log"
+	"net/url"
+	"os"
 	"os/exec"
 	"regexp"
 	"strconv"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/tardisx/gropple/config"
@@ -17,7 +21,7 @@ type Download struct {
 	Id              int                    `json:"id"`
 	Url             string                 `json:"url"`
 	PopupUrl        string                 `json:"popup_url"`
-	Pid             int                    `json:"pid"`
+	Process         *os.Process            `json:"-"`
 	ExitCode        int                    `json:"exit_code"`
 	State           string                 `json:"state"`
 	DownloadProfile config.DownloadProfile `json:"download_profile"`
@@ -28,33 +32,43 @@ type Download struct {
 	Percent         float32                `json:"percent"`
 	Log             []string               `json:"log"`
 	Config          *config.Config
+	mutex           sync.Mutex
 }
 
 type Downloads []*Download
 
+var CanStopDownload = false
+
+var downloadId int32 = 0
+
 // StartQueued starts any downloads that have been queued, we would not exceed
 // maxRunning. If maxRunning is 0, there is no limit.
 func (dls Downloads) StartQueued(maxRunning int) {
-	active := 0
+	active := make(map[string]int)
-	queued := 0
 
 	for _, dl := range dls {
+
+		dl.mutex.Lock()
+
 		if dl.State == "downloading" {
-			active++
+			active[dl.domain()]++
-		}
-		if dl.State == "queued" {
-			queued++
 		}
+		dl.mutex.Unlock()
+
 	}
 
-	// there is room, so start one
-	if queued > 0 && (active < maxRunning || maxRunning == 0) {
 	for _, dl := range dls {
-			if dl.State == "queued" {
+
+		dl.mutex.Lock()
+
+		if dl.State == "queued" && (maxRunning == 0 || active[dl.domain()] < maxRunning) {
 			dl.State = "downloading"
+			active[dl.domain()]++
+			log.Printf("Starting download for id:%d (%s)", dl.Id, dl.Url)
+			dl.mutex.Unlock()
 			go func() { dl.Begin() }()
-				return
+		} else {
-			}
+			dl.mutex.Unlock()
 		}
 	}
 
@@ -65,29 +79,84 @@ func (dls Downloads) StartQueued(maxRunning int) {
 func (dls Downloads) Cleanup() Downloads {
 	newDLs := Downloads{}
 	for _, dl := range dls {
+
+		dl.mutex.Lock()
+
 		if dl.Finished && time.Since(dl.FinishedTS) > time.Duration(time.Hour) {
 			// do nothing
 		} else {
 			newDLs = append(newDLs, dl)
 		}
+		dl.mutex.Unlock()
+
 	}
 	return newDLs
 }
 
 // Queue queues a download
 func (dl *Download) Queue() {
+
+	dl.mutex.Lock()
+	defer dl.mutex.Unlock()
+
 	dl.State = "queued"
+
+}
+
+func NewDownload(conf *config.Config, url string) *Download {
+	atomic.AddInt32(&downloadId, 1)
+	dl := Download{
+		Config: conf,
+
+		Id:       int(downloadId),
+		Url:      url,
+		PopupUrl: fmt.Sprintf("/fetch/%d", int(downloadId)),
+		State:    "choose profile",
+		Finished: false,
+		Eta:      "?",
+		Percent:  0.0,
+		Log:      make([]string, 0, 1000),
+	}
+	return &dl
+}
+
+func (dl *Download) Stop() {
+	if !CanStopDownload {
+		log.Print("attempted to stop download on a platform that it is not currently supported on - please report this as a bug")
+		os.Exit(1)
+	}
+	log.Printf("stopping the download")
+	dl.mutex.Lock()
+	dl.Log = append(dl.Log, "aborted by user")
+	defer dl.mutex.Unlock()
+	dl.Process.Kill()
+}
+
+func (dl *Download) domain() string {
+
+	// note that we expect to already have the mutex locked by the caller
+	url, err := url.Parse(dl.Url)
+	if err != nil {
+		log.Printf("Unknown domain for url: %s", dl.Url)
+		return "unknown"
+	}
+
+	return url.Hostname()
+
 }
 
 // Begin starts a download, by starting the command specified in the DownloadProfile.
 // It blocks until the download is complete.
 func (dl *Download) Begin() {
+
+	dl.mutex.Lock()
+
 	dl.State = "downloading"
 	cmdSlice := []string{}
 	cmdSlice = append(cmdSlice, dl.DownloadProfile.Args...)
 
-	// only add the url if it's not empty. This helps us with testing
+	// only add the url if it's not empty or an example URL. This helps us with testing
-	if dl.Url != "" {
+	if !(dl.Url == "" || strings.Contains(dl.domain(), "example.org")) {
 		cmdSlice = append(cmdSlice, dl.Url)
 	}
 
@@ -112,6 +181,7 @@ func (dl *Download) Begin() {
 		return
 	}
 
+	log.Printf("Executing command: %v", cmd)
 	err = cmd.Start()
 	if err != nil {
 		dl.State = "failed"
@@ -120,10 +190,12 @@ func (dl *Download) Begin() {
 		dl.Log = append(dl.Log, fmt.Sprintf("error starting command '%s': %v", dl.DownloadProfile.Command, err))
 		return
 	}
-	dl.Pid = cmd.Process.Pid
+	dl.Process = cmd.Process
 
 	var wg sync.WaitGroup
 
+	dl.mutex.Unlock()
+
 	wg.Add(2)
 	go func() {
 		defer wg.Done()
@@ -138,6 +210,9 @@ func (dl *Download) Begin() {
 	wg.Wait()
 	cmd.Wait()
 
+	dl.mutex.Lock()
+	log.Printf("Process finished for id: %d (%v)", dl.Id, cmd)
+
 	dl.State = "complete"
 	dl.Finished = true
 	dl.FinishedTS = time.Now()
@@ -146,6 +221,7 @@ func (dl *Download) Begin() {
 	if dl.ExitCode != 0 {
 		dl.State = "failed"
 	}
+	dl.mutex.Unlock()
 
 }
 
@@ -164,9 +240,13 @@ func (dl *Download) updateDownload(r io.Reader) {
 					continue
 				}
 
+				dl.mutex.Lock()
+
 				// append the raw log
 				dl.Log = append(dl.Log, l)
 
+				dl.mutex.Unlock()
+
 				// look for the percent and eta and other metadata
 				dl.updateMetadata(l)
 			}
@@ -179,6 +259,10 @@ func (dl *Download) updateDownload(r io.Reader) {
 
 func (dl *Download) updateMetadata(s string) {
 
+	dl.mutex.Lock()
+
+	defer dl.mutex.Unlock()
+
 	// [download]  49.7% of ~15.72MiB at  5.83MiB/s ETA 00:07
 	etaRE := regexp.MustCompile(`download.+ETA +(\d\d:\d\d(?::\d\d)?)$`)
 	matches := etaRE.FindStringSubmatch(s)

download/download_test.go

@@ -2,6 +2,7 @@ package download
 
 import (
 	"testing"
+	"time"
 
 	"github.com/tardisx/gropple/config"
 )
@@ -65,35 +66,76 @@ func TestUpdateMetadata(t *testing.T) {
 // [download] 100% of 4.64MiB in 00:00
 // [ffmpeg] Merging formats into "Halo Infinite Flight 4K Gameplay-wi7Agv1M6PY.mp4"
 
-// This test is a bit broken, because StartQueued immediately starts the queued
-// download, it
 func TestQueue(t *testing.T) {
-	conf := config.TestConfig()
+	cs := config.ConfigService{}
+	cs.LoadTestConfig()
+	conf := cs.Config
 
-	new1 := Download{Id: 1, State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
+	new1 := Download{Id: 1, Url: "http://sub.example.org/foo1", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
-	new2 := Download{Id: 2, State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
+	new2 := Download{Id: 2, Url: "http://sub.example.org/foo2", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
-	new3 := Download{Id: 3, State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
+	new3 := Download{Id: 3, Url: "http://sub.example.org/foo3", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
-	new4 := Download{Id: 4, State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
+	new4 := Download{Id: 4, Url: "http://example.org/", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
 
 	dls := Downloads{&new1, &new2, &new3, &new4}
 	dls.StartQueued(1)
+	time.Sleep(time.Millisecond * 100)
 	if dls[0].State == "queued" {
 		t.Error("#1 was not started")
 	}
 	if dls[1].State != "queued" {
 		t.Error("#2 is not queued")
 	}
+	if dls[3].State == "queued" {
+		t.Error("#4 is not started")
+	}
 
 	// this should start no more, as one is still going
 	dls.StartQueued(1)
+	time.Sleep(time.Millisecond * 100)
 	if dls[1].State != "queued" {
 		t.Error("#2 was started when it should not be")
 	}
 
 	dls.StartQueued(2)
+	time.Sleep(time.Millisecond * 100)
 	if dls[1].State == "queued" {
 		t.Error("#2 was not started but it should be")
 
 	}
 
+	dls.StartQueued(2)
+	time.Sleep(time.Millisecond * 100)
+	if dls[3].State == "queued" {
+		t.Error("#4 was not started but it should be")
+	}
+
+	// reset them all
+	dls[0].State = "queued"
+	dls[1].State = "queued"
+	dls[2].State = "queued"
+	dls[3].State = "queued"
+
+	dls.StartQueued(0)
+	time.Sleep(time.Millisecond * 100)
+
+	// they should all be going
+	if dls[0].State == "queued" || dls[1].State == "queued" || dls[2].State == "queued" || dls[3].State == "queued" {
+		t.Error("none should be queued")
+	}
+
+	// reset them all
+	dls[0].State = "queued"
+	dls[1].State = "queued"
+	dls[2].State = "queued"
+	dls[3].State = "queued"
+
+	dls.StartQueued(2)
+	time.Sleep(time.Millisecond * 100)
+
+	// first two should be running, third not (same domain) and 4th running (different domain)
+	if dls[0].State == "queued" || dls[1].State == "queued" || dls[2].State != "queued" || dls[3].State == "queued" {
+		t.Error("incorrect queued")
+
+	}
+
 }

main.go

@@ -21,9 +21,9 @@ import (
 
 var downloads download.Downloads
 var downloadId = 0
-var conf *config.Config
+var configService *config.ConfigService
 
-var versionInfo = version.Info{CurrentVersion: "v0.5.3"}
+var versionInfo = version.Info{CurrentVersion: "v0.5.5"}
 
 //go:embed web
 var webFS embed.FS
@@ -39,20 +39,31 @@ type errorResponse struct {
 }
 
 func main() {
-	if !config.ConfigFileExists() {
+	log.Printf("Starting gropple %s - https://github.com/tardisx/gropple", versionInfo.CurrentVersion)
-		log.Print("No config file - creating default config")
+
-		conf = config.DefaultConfig()
+	configService = &config.ConfigService{}
-		conf.WriteConfig()
+	configService.DetermineConfigDir()
-	} else {
+	exists, err := configService.ConfigFileExists()
-		loadedConfig, err := config.LoadConfig()
 	if err != nil {
 		log.Fatal(err)
 	}
-		conf = loadedConfig
+	if !exists {
+		log.Print("No config file - creating default config")
+		configService.LoadDefaultConfig()
+		configService.WriteConfig()
+		log.Printf("Configuration written to %s", configService.ConfigPath)
+	} else {
+		err := configService.LoadConfig()
+		if err != nil {
+			log.Fatal(err)
+		}
+		log.Printf("Configuration loaded from %s", configService.ConfigPath)
+
 	}
 
 	r := mux.NewRouter()
 	r.HandleFunc("/", homeHandler)
+	r.HandleFunc("/static/{filename}", staticHandler)
 	r.HandleFunc("/config", configHandler)
 	r.HandleFunc("/fetch", fetchHandler)
 	r.HandleFunc("/fetch/{id}", fetchHandler)
@@ -68,7 +79,7 @@ func main() {
 
 	srv := &http.Server{
 		Handler: r,
-		Addr:    fmt.Sprintf(":%d", conf.Server.Port),
+		Addr:    fmt.Sprintf(":%d", configService.Config.Server.Port),
 		// Good practice: enforce timeouts for servers you create!
 		WriteTimeout: 5 * time.Second,
 		ReadTimeout:  5 * time.Second,
@@ -86,15 +97,15 @@ func main() {
 	// old entries
 	go func() {
 		for {
-			downloads.StartQueued(conf.Server.MaximumActiveDownloads)
+			downloads.StartQueued(configService.Config.Server.MaximumActiveDownloads)
 			downloads = downloads.Cleanup()
 			time.Sleep(time.Second)
 		}
 	}()
 
-	log.Printf("starting gropple %s - https://github.com/tardisx/gropple", versionInfo.CurrentVersion)
+	log.Printf("Visit %s for details on installing the bookmarklet and to check status", configService.Config.Server.Address)
-	log.Printf("go to %s for details on installing the bookmarklet and to check status", conf.Server.Address)
 	log.Fatal(srv.ListenAndServe())
+
 }
 
 // versionRESTHandler returns the version information, if we have up-to-date info from github
@@ -111,7 +122,7 @@ func versionRESTHandler(w http.ResponseWriter, r *http.Request) {
 func homeHandler(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusOK)
 
-	bookmarkletURL := fmt.Sprintf("javascript:(function(f,s,n,o){window.open(f+encodeURIComponent(s),n,o)}('%s/fetch?url=',window.location,'yourform','width=%d,height=%d'));", conf.Server.Address, conf.UI.PopupWidth, conf.UI.PopupHeight)
+	bookmarkletURL := fmt.Sprintf("javascript:(function(f,s,n,o){window.open(f+encodeURIComponent(s),n,o)}('%s/fetch?url=',window.location,'yourform','width=%d,height=%d'));", configService.Config.Server.Address, configService.Config.UI.PopupWidth, configService.Config.UI.PopupHeight)
 
 	t, err := template.ParseFS(webFS, "web/layout.tmpl", "web/menu.tmpl", "web/index.html")
 	if err != nil {
@@ -127,7 +138,7 @@ func homeHandler(w http.ResponseWriter, r *http.Request) {
 	info := Info{
 		Downloads:      downloads,
 		BookmarkletURL: template.URL(bookmarkletURL),
-		Config:         conf,
+		Config:         configService.Config,
 	}
 
 	err = t.ExecuteTemplate(w, "layout", info)
@@ -136,6 +147,25 @@ func homeHandler(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+// staticHandler handles requests for static files
+func staticHandler(w http.ResponseWriter, r *http.Request) {
+	vars := mux.Vars(r)
+
+	filename := vars["filename"]
+	if strings.Index(filename, ".js") == len(filename)-3 {
+		f, err := webFS.Open("web/" + filename)
+		if err != nil {
+			log.Printf("error accessing %s - %v", filename, err)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+		io.Copy(w, f)
+		return
+	}
+	w.WriteHeader(http.StatusNotFound)
+}
+
 // configHandler returns the configuration page
 func configHandler(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusOK)
@@ -160,7 +190,7 @@ func configRESTHandler(w http.ResponseWriter, r *http.Request) {
 		if err != nil {
 			panic(err)
 		}
-		err = conf.UpdateFromJSON(b)
+		err = configService.Config.UpdateFromJSON(b)
 
 		if err != nil {
 			errorRes := errorResponse{Success: false, Error: err.Error()}
@@ -169,9 +199,9 @@ func configRESTHandler(w http.ResponseWriter, r *http.Request) {
 			w.Write(errorResB)
 			return
 		}
-		conf.WriteConfig()
+		configService.WriteConfig()
 	}
-	b, _ := json.Marshal(conf)
+	b, _ := json.Marshal(configService.Config)
 	w.Write(b)
 }
 
@@ -223,7 +253,7 @@ func fetchInfoOneRESTHandler(w http.ResponseWriter, r *http.Request) {
 
 			if thisReq.Action == "start" {
 				// find the profile they asked for
-				profile := conf.ProfileCalled(thisReq.Profile)
+				profile := configService.Config.ProfileCalled(thisReq.Profile)
 				if profile == nil {
 					panic("bad profile name?")
 				}
@@ -236,6 +266,14 @@ func fetchInfoOneRESTHandler(w http.ResponseWriter, r *http.Request) {
 				w.Write(succResB)
 				return
 			}
+
+			if thisReq.Action == "stop" {
+				thisDownload.Stop()
+				succRes := successResponse{Success: true, Message: "download stopped"}
+				succResB, _ := json.Marshal(succRes)
+				w.Write(succResB)
+				return
+			}
 		}
 
 		// just a get, return the object
@@ -268,7 +306,7 @@ func fetchHandler(w http.ResponseWriter, r *http.Request) {
 					panic(err)
 				}
 
-				templateData := map[string]interface{}{"dl": dl, "config": conf}
+				templateData := map[string]interface{}{"dl": dl, "config": configService.Config, "canStop": download.CanStopDownload}
 
 				err = t.ExecuteTemplate(w, "layout", templateData)
 				if err != nil {
@@ -290,28 +328,15 @@ func fetchHandler(w http.ResponseWriter, r *http.Request) {
 	} else {
 
 		// check the URL for a sudden but inevitable betrayal
-		if strings.Contains(url[0], conf.Server.Address) {
+		if strings.Contains(url[0], configService.Config.Server.Address) {
 			w.WriteHeader(400)
-			fmt.Fprint(w, "you musn't gropple your gropple :-)")
+			fmt.Fprint(w, "you mustn't gropple your gropple :-)")
 			return
 		}
 
 		// create the record
-		// XXX should be atomic!
+		newDownload := download.NewDownload(configService.Config, url[0])
-		downloadId++
+		downloads = append(downloads, newDownload)
-		newDownload := download.Download{
-			Config: conf,
-
-			Id:       downloadId,
-			Url:      url[0],
-			PopupUrl: fmt.Sprintf("/fetch/%d", downloadId),
-			State:    "choose profile",
-			Finished: false,
-			Eta:      "?",
-			Percent:  0.0,
-			Log:      make([]string, 0, 1000),
-		}
-		downloads = append(downloads, &newDownload)
 		// XXX atomic ^^
 
 		newDownload.Log = append(newDownload.Log, "start of log...")
@@ -325,7 +350,7 @@ func fetchHandler(w http.ResponseWriter, r *http.Request) {
 			panic(err)
 		}
 
-		templateData := map[string]interface{}{"dl": newDownload, "config": conf}
+		templateData := map[string]interface{}{"dl": newDownload, "config": configService.Config, "canStop": download.CanStopDownload}
 
 		err = t.ExecuteTemplate(w, "layout", templateData)
 		if err != nil {

version/version.go

@@ -63,8 +63,6 @@ func (i *Info) canUpgrade() bool {
 		return false
 	}
 
-	log.Printf("We are %s, github is %s", i.CurrentVersion, i.GithubVersion)
-
 	if !semver.IsValid(i.CurrentVersion) {
 		log.Printf("current version %s is invalid", i.CurrentVersion)
 	}

web/config.html

@@ -11,7 +11,7 @@
 
     <div class="pure-g">
 
-        <div class="pure-u-md-1-2 pure-u-1">
+        <div class="pure-u-md-1-2 pure-u-1 l-box">
    
             <form class="pure-form pure-form-stacked gropple-config">
                 <fieldset>
@@ -33,9 +33,9 @@
                     <input type="text" id="config-server-downloadpath" placeholder="path" class="input-long" x-model="config.server.download_path" />
                     <span class="pure-form-message">The path on the server to download files to.</span>
 
-                    <label for="config-server-max-downloads">Maximum active downloads</label>
+                    <label for="config-server-max-downloads">Maximum active downloads per domain</label>
                     <input type="text" id="config-server-max-downloads" placeholder="2" class="input-long" x-model.number="config.server.maximum_active_downloads_per_domain" />
-                    <span class="pure-form-message">How many downloads can be simultaneously active. Use '0' for no limit.</span>
+                    <span class="pure-form-message">How many downloads can be simultaneously active. Use '0' for no limit. This limit is applied per domain that you download from.</span>
 
                     <legend>UI</legend>
 
@@ -53,7 +53,7 @@
             </form>
 
         </div>
-        <div class="pure-u-md-1-2 pure-u-1">
+        <div class="pure-u-md-1-2 pure-u-1 l-box">
             <form class="pure-form gropple-config">
                 <fieldset>
 
@@ -91,6 +91,7 @@
                             </template>
 
                             <button class="pure-button button-add" href="#" @click.prevent="profile.args.push('');">add arg</button>
+                            <span class="pure-form-message">Arguments for the command. Note that the shell is not used, so there is no need to quote or escape arguments, including those with spaces.</span>
 
                             <hr>
 

web/layout.tmpl

@@ -3,11 +3,17 @@
 <head>
     <meta charset="utf-8">
     <title>gropple</title>
-    <script src="//unpkg.com/alpinejs" defer></script>
+    <script src="/static/alpine.min.js" defer></script>
     <meta name="viewport" content="width=device-width, initial-scale=1">
     <link rel="stylesheet" href="https://unpkg.com/purecss@2.0.6/build/pure-min.css" integrity="sha384-Uu6IeWbM+gzNVXJcM9XV3SohHtmWE+3VGi496jvgX1jyvDTXfdK+rfZc8C1Aehk5" crossorigin="anonymous">
     <link rel="stylesheet" href="https://unpkg.com/purecss@2.0.6/build/grids-responsive-min.css">
     <style>
+        .pure-g > div {
+          box-sizing: border-box;
+        }
+        .l-box {
+          padding: 2em;
+         }
         pre {
           font-size: 60%;
           height: 100px;
@@ -51,6 +57,7 @@
         }
         .error {
           color: red;
+          font-size: 150%;
         }
         .success {
           color: green;

web/popup.html

@@ -18,8 +18,12 @@
             <tr><th>state</th><td x-text="state"></td></tr>
             <tr><th>progress</th><td x-text="percent"></td></tr>
             <tr><th>ETA</th><td x-text="eta"></td></tr>
+            
         </table>
         <p>You can close this window and your download will continue. Check the <a href="/" target="_gropple_status">Status page</a> to see all downloads in progress.</p>
+        {{ if .canStop }}
+        <button x-show="state=='downloading'" class="pure-button" @click="stop()">stop</button>
+        {{ end }}
         <div>
             <h4>Logs</h4>
             <pre x-text="log">
@@ -50,6 +54,18 @@
                     console.log(info)
                 })
             },
+            stop() {
+                let op = {
+                   method: 'POST',
+                   body: JSON.stringify({action: 'stop'}),
+                   headers: { 'Content-Type': 'application/json' }
+                };
+                fetch('/rest/fetch/{{ .dl.Id }}', op)
+                .then(response => response.json())
+                .then(info => {
+                    console.log(info)
+                })
+            },
             fetch_data() {
                 fetch('/rest/fetch/{{ .dl.Id }}')
                 .then(response => response.json())