Make 'sh:' directives a bit more usable (#17)

* Use %pistol-filename% and shellescape for sh:
* Somewhat standardize release procedure
* Add tests and make it surely work
* README: mention debugging

Makefile

@@ -1,8 +1,47 @@
-pistol:
-	go build ./cmd/pistol
-all: pistol
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, version 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+NAME := pistol
+VERSION := v0.1 (2020-04-17) (breaking release, see README)
+version := $(word 1, $(VERSION))
+
+build:
+	go build -ldflags "-X 'main.VERSION=$(VERSION)'" ./cmd/pistol
 
 install:
-	go install ./cmd/pistol
+	go install -ldflags "-X 'main.VERSION=$(VERSION)'" ./cmd/pistol
+
+# requires: bat (https://github.com/sharkdp/bat), elinks
+test: pistol
+	@echo -------------------
+	@echo fpath
+	@echo -------------------
+	@./pistol --config tests/config tests/fpath-no-sh
+	@tput sgr0
+	@echo -------------------
+	@echo fpath + sh:
+	@echo -------------------
+	@./pistol --config tests/config tests/fpath-with-sh
+	@tput sgr0
+	@echo -------------------
+	@echo mimetype
+	@echo -------------------
+	@./pistol --config tests/config tests/mimetype-no-sh
+	@tput sgr0
+	@echo -------------------
+	@echo mimetype + sh:
+	@echo -------------------
+	@./pistol --config tests/config tests/mimetype-with-sh
+
+deps:
+	go get github.com/c4milo/github-release
+	go get github.com/mitchellh/gox
+
+changelog:
+	@latest_tag=$$(git describe --tags `git rev-list --tags --max-count=1`); \
+	comparison="$$latest_tag..HEAD"; \
+	if [ -z "$$latest_tag" ]; then comparison=""; fi; \
+	git --no-pager log $$comparison --oneline --no-merges
 
-.PHONY: pistol
+.PHONY: build install changelog

README.md

@@ -1,5 +1,27 @@
 # Pistol
 
+## NOTE TO EXISTING USERS
+
+If you've updated to [v0.1](https://github.com/doronbehar/pistol/releases) or
+higher, you may experience errors with custom commands set in your config,
+similar to:
+
+```
+[bat error]: '%pistol-filename%': No such file or directory (os error 2)
+```
+
+Basically, after hitting [issue #16](https://github.com/doronbehar/pistol/issues/16),
+I realised that the old way Pistol substituted the file name in your config was
+not scalable. So now, please replace every occurrence of `%s` with
+`%pistol-filename%`. Or with:
+
+```sh
+sed -i 's/%s/%pistol-filename%/g ~/.config/pistol/pistol.conf
+```
+
+If you want to know more details, read
+[this](https://github.com/doronbehar/pistol/issues/16#issuecomment-614471555).
+
 ## Introduction
 
 Pistol is a file previewer for command line file managers such as
@@ -8,10 +30,12 @@ intended to replace the file previewer
 [`scope.sh`](https://github.com/ranger/ranger/blob/v1.9.2/ranger/data/scope.sh)
 commonly used with them.
 
-`scope.sh` is a Bash script that uses `case` switches and external programs to decide how to preview every file it encounters. It knows how to handle every file according to it's [MIME
-type](https://en.wikipedia.org/wiki/Media_type) and/or file extension using
-`case` switches and external programs. This design makes it hard to configure
-/ maintain and it makes it slow for startup and heavy when running.
+`scope.sh` is a Bash script that uses `case` switches and external programs to
+decide how to preview every file it encounters. It knows how to handle every
+file according to it's [MIME type](https://en.wikipedia.org/wiki/Media_type)
+and/or file extension using `case` switches and external programs. This design
+makes it hard to configure / maintain and it makes it slow for startup and
+heavy when running.
 
 Pistol is a Go program (with (almost) 0 dependencies) and it's MIME type detection is
 internal. Moreover, it features native preview support for almost any archive
@@ -34,7 +58,8 @@ executable might be:
 ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=a34861a1ae5358dc1079bc239df9dfe4830a8403, for GNU/Linux 3.2.0, not stripped
 ```
 
-This feature is available out of the box just like the previews for the common mime types mentioned above.
+This feature is available out of the box just like the previews for the common
+mime types mentioned above.
 
 ### A Note on MIME type Detection
 
@@ -104,17 +129,17 @@ env GO111MODULE=on go get -u github.com/doronbehar/pistol/cmd/pistol
 
 ```
 $ pistol --help
-Usage: pistol OPTIONS <file>
+Usage: pistol OPTIONS [<file> ...]
 
 OPTIONS
 
--c, --config <config>  configuration file to use (defaults to ~/.config/pistol/pistol.conf)
--h, --help             print help and exit
--v, --verbosity        increase verbosity
+-V, --version               Print version date and exit
+-c, --config <config file>  configuration file to use (defaults to ~/.config/pistol/pistol.conf)
+-h, --help                  print help and exit
 
 ARGUMENTS
 
-file                   the file to preview
+file                        the file to preview
 ```
 
 ### Integrations
@@ -165,8 +190,8 @@ argument is interpreted as a file path regex, such as:
 `/var/src/.*/README.md`.
 
 On every line, whether you used `fpath` or not, the next arguments are the
-command's arguments, where `%s` is substituted by `pistol` to the file at
-question. You'll see more examples in the following sections.
+command's arguments, where `%pistol-filename%` is substituted by `pistol` to
+the file at question. You'll see more examples in the following sections.
 
 Both regular expressions (for file paths and for mime types) are interpreted by
 the [built-in library's `regexp.match`](https://golang.org/pkg/regexp/#Match).
@@ -183,7 +208,7 @@ of [bat](https://github.com/sharkdp/bat)'s, you'd put the following in your
 `pistol.conf`:
 
 ```
-text/* bat --paging=never --color=always %s
+text/* bat --paging=never --color=always %pistol-filename%
 ```
 
 Naturally, your configuration file overrides the internal previewers.
@@ -192,23 +217,23 @@ Here's another example which features [w3m](http://w3m.sourceforge.net/) as an
 HTML previewer:
 
 ```
-text/html w3m -T text/html -dump %s
+text/html w3m -T text/html -dump %pistol-filename%
 ```
 
 And here's an example that leverages `ls` for printing directories' contents:
 
 ```
-inode/directory ls -l --color %s
+inode/directory ls -l --color %pistol-filename%
 ```
 
 #### Matching File Path
 
-For example, say you wish to preview all files that reside in a certain `./bin` directory with
-[bat](https://github.com/sharkdp/bat)'s syntax highlighting for
+For example, say you wish to preview all files that reside in a certain `./bin`
+directory with [bat](https://github.com/sharkdp/bat)'s syntax highlighting for
 bash. You could use:
 
 ```
-fpath /var/src/my-bash-project/bin/[^/]+$ bat --map-syntax :bash --paging=never --color=always %s
+fpath /var/src/my-bash-project/bin/[^/]+$ bat --map-syntax :bash --paging=never --color=always %pistol-filename%
 ```
 
 #### A Note on RegEx matching
@@ -237,7 +262,7 @@ Pistol is pretty dumb when it parses your config, it splits all line by spaces,
 meaning that e.g:
 
 ```config
-application/json jq '.' %s
+application/json jq '.' %pistol-filename%
 ```
 
 This will result in an error by [`jq`](https://github.com/stedolan/jq):
@@ -254,7 +279,7 @@ around `.`. However, Pistol is not smarter then your shell because if you'd try
 for example:
 
 ```config
-application/json jq '.[] | .' %s
+application/json jq '.[] | .' %pistol-filename%
 ```
 
 That would be equivalent to running in the typical shell:
@@ -268,7 +293,7 @@ it just splits words by spaces. Hence, to overcome this disability, you can
 use:
 
 ```config
-application/json sh: jq '.' %s
+application/json sh: jq '.' %pistol-filename%
 ```
 
 Thanks to the `sh:` keyword at the beginning of the command's definition, the
@@ -281,7 +306,7 @@ quotes are escaped or not used at all inside `command`.
 More over, with `sh:` you can use shell pipes:
 
 ```config
-fpath .*.md$ sh: bat --paging=never --color=always %s | head -8
+fpath .*.md$ sh: bat --paging=never --color=always %pistol-filename% | head -8
 ```
 
 ### Environmental Variables
@@ -335,6 +360,17 @@ a specific style set `PISTOL_CHROMA_STYLE` in your environment, e.g:
 export PISTOL_CHROMA_STYLE=monokai
 ```
 
+
+## Debugging
+
+Can't figure out way does Pistol acts the way he does? You can run pistol with:
+
+```sh
+env PISTOL_DEBUG=1 pistol test-file
+```
+
+And you should be able to see messages that may give you a clue.
+
 ## Footnotes
 
 <b id="f1">1</b> Considering Pistol's indirect dependence on

cmd/pistol/main.go

@@ -9,6 +9,10 @@ import (
 	"github.com/adrg/xdg"
 )
 
+var (
+	VERSION string
+)
+
 func main() {
 	// Setup logger
 	log.SetFlags(0)
@@ -22,7 +26,7 @@ func main() {
 	cmd.Parse(os.Args)
 
 	if cmd.IsOptionSet("version") {
-		print("v0.0.5 (2020-04-13) (corona edition)\n")
+		print(VERSION, "\n")
 		os.Exit(0)
 	}
 

go.mod

@@ -21,5 +21,6 @@ require (
 	github.com/sirupsen/logrus v1.4.2
 	github.com/stretchr/testify v1.4.0 // indirect
 	golang.org/x/sys v0.0.0-20200219091948-cb0a6d8edb6c // indirect
+	gopkg.in/alessio/shellescape.v1 v1.0.0-20170105083845-52074bc9df61
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
 )

go.sum

@@ -98,6 +98,8 @@ golang.org/x/sys v0.0.0-20200219091948-cb0a6d8edb6c h1:jceGD5YNJGgGMkJz79agzOln1
 golang.org/x/sys v0.0.0-20200219091948-cb0a6d8edb6c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+gopkg.in/alessio/shellescape.v1 v1.0.0-20170105083845-52074bc9df61 h1:8ajkpB4hXVftY5ko905id+dOnmorcS2CHNxxHLLDcFM=
+gopkg.in/alessio/shellescape.v1 v1.0.0-20170105083845-52074bc9df61/go.mod h1:IfMagxm39Ys4ybJrDb7W3Ob8RwxftP0Yy+or/NVz1O8=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

previewer.go

@@ -5,11 +5,13 @@ import (
 	"os"
 	"io"
 	"os/exec"
-	"fmt"
+	// "fmt"
 	"strings"
 	"regexp"
 	"path/filepath"
+	"errors"
 
+	she "gopkg.in/alessio/shellescape.v1"
 	log "github.com/sirupsen/logrus"
 	"github.com/doronbehar/pistol/internal_writers"
 	"github.com/rakyll/magicmime"
@@ -65,15 +67,17 @@ func NewPreviewer(filePath, configPath string) (Previewer, error) {
 		}
 		if match && len(def) > 1 {
 			p.command = def[1]
-			for _, arg := range def[2:] {
-				if match, _ := regexp.MatchString("%s", arg); match {
-					p.args = append(p.args, fmt.Sprintf(arg, filePath))
-				} else {
-					p.args = append(p.args, arg)
-				}
-			}
+			p.args = def[2:]
 			return p, nil
 		}
+		match, err = regexp.MatchString("^#", def[0])
+		if err != nil {
+			return p, err
+		}
+		if match {
+			// This is a comment, line skipped
+			continue
+		}
 		// Test if fpath keyword is used at the beginning, indicating it's a
 		// file path match we should be looking for
 		if def[0] == "fpath" {
@@ -98,14 +102,7 @@ func NewPreviewer(filePath, configPath string) (Previewer, error) {
 		if match {
 			log.Infof("matched file path against absFpath: %s", absFpath)
 			p.command = def[2]
-			for _, arg := range def[3:] {
-				if match, _ := regexp.MatchString("%s", arg); match {
-					// Question: Should we use filePath instead here?
-					p.args = append(p.args, fmt.Sprintf(arg, absFpath))
-				} else {
-					p.args = append(p.args, arg)
-				}
-			}
+			p.args = def[3:]
 			return p, nil
 		}
 	}
@@ -116,17 +113,31 @@ func NewPreviewer(filePath, configPath string) (Previewer, error) {
 func (p *Previewer) Write(w io.Writer) (error) {
 	// if a match was encountered when the configuration file was read
 	if p.command != "" {
+		if match, _ := regexp.MatchString("%pistol-filename%", strings.Join(p.args, " ")); !match {
+			return errors.New("no %pistol-filename% found in definition command")
+		}
+		var replStr string
+		if p.command == "sh:" {
+			replStr = she.Quote(p.filePath)
+		} else {
+			replStr = p.filePath
+		}
 		var cmd *exec.Cmd
+		var argsOut []string
+		for _, arg := range p.args {
+			argsOut = append(argsOut, strings.ReplaceAll(arg, "%pistol-filename%", replStr))
+		}
 		if p.command == "sh:" {
-			log.Infof("previewer's command is (shell interpreted): %s\n", p.args[0:])
-			cmd = exec.Command("sh", "-c", strings.Join(p.args[0:], " "))
+			log.Infof("previewer's command is (shell interpreted): %#v\n", argsOut)
+			cmd = exec.Command("sh", "-c", strings.Join(argsOut, " "))
 		} else {
-			log.Infof("previewer's command is %s %s\n", p.command, strings.Join(p.args, " "))
-			cmd = exec.Command(p.command, p.args...)
+			log.Infof("previewer's command is (no shell) %#v with args: %#v\n", p.command, argsOut)
+			cmd = exec.Command(p.command, argsOut...)
 		}
 		cmd.Stdout = w
 		cmd.Stderr = os.Stderr
 		if err := cmd.Start(); err != nil {
+			log.Fatalf("We've had issues running your command: %v, %s", p.command, p.args)
 			return err
 		}
 		cmd.Wait()

tests/config

@@ -0,0 +1,8 @@
+# detects: fpath, sh: no
+fpath .*/tests/fpath-no-sh$ bat --map-syntax :Markdown --paging=never --style=numbers --color=always %pistol-filename%
+# detects: fpath, sh: yes
+fpath .*/tests/fpath-with-sh$ sh: bat --map-syntax :Markdown --paging=never --style=numbers --color=always %pistol-filename% | head -2
+# detects: mimetype, sh: no
+text/html elinks -dump -dump-color-mode 1 %pistol-filename%
+# detects: mimetype, sh: yes
+text/plain sh: bat --map-syntax :Markdown --paging=never --style=numbers --color=always %pistol-filename% | head -1

tests/fpath-no-sh

@@ -0,0 +1 @@
+fpath detected, shell is not used

tests/fpath-with-sh

@@ -0,0 +1,14 @@
+fpath detected - line numbers should appear,
+Shell is used - There should be more text after this
+
+Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec vehicula eros a
+orci eleifend, aliquet venenatis arcu volutpat. Donec eget volutpat tellus.
+Duis vestibulum lobortis tortor, eget euismod justo semper sed. Mauris id
+imperdiet diam, et sodales odio. Vestibulum placerat mi ante, pretium maximus
+tellus lobortis sit amet. Fusce in magna at erat auctor interdum. Vivamus id mi
+malesuada, luctus magna at, dignissim turpis. Sed finibus volutpat felis, eget
+lacinia mi. Nam accumsan congue ante sed venenatis. Duis imperdiet vestibulum
+massa, id dignissim metus sollicitudin ac. Suspendisse tincidunt a dolor non
+hendrerit. Phasellus non velit nulla. Orci varius natoque penatibus et magnis
+dis parturient montes, nascetur ridiculus mus. Suspendisse vulputate eleifend
+pellentesque. Aenean feugiat ullamcorper scelerisque.