sloum/bombadillo
sloum
/
bombadillo
5
27
Fork 7
Code Issues 18 Pull Requests 1 Releases 9 Activity

Makes local protocol navigable #94

Manually merged
sloum merged 3 commits from improved-local into develop 2019-11-17 23:05:25 +00:00
Conversation 7 Commits 3 Files Changed 3 +46 -14
3 changed files with 46 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
client.go
View File

@ -963,13 +963,13 @@ func (c *client) handleTelnet(u Url) {
}
func (c *client) handleLocal(u Url) {
content, err := local.Open(u.Resource)
content, links, err := local.Open(u.Resource)
if err != nil {
c.SetMessage(err.Error(), true)
c.DrawMessage()
return
}
pg := MakePage(u, content, []string{})
pg := MakePage(u, content, links)
pg.WrapContent(c.Width - 1)
c.PageState.Add(pg)
c.SetPercentRead()

54
local/local.go
View File

@ -4,39 +4,71 @@ import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"sort"
"strings"
)
func Open(address string) (string, error) {
func Open(address string) (string, []string, error) {
links := make([]string, 0, 10)
if !pathExists(address) {
return "", fmt.Errorf("Invalid system path: %s", address)
return "", links, fmt.Errorf("Invalid system path: %s", address)
}
file, err := os.Open(address)
if err != nil {
return "", fmt.Errorf("Unable to open file: %s", address)
return "", links, err
}
defer file.Close()
if pathIsDir(address) {
fileList, err := file.Readdirnames(0)
offset := 1
fileList, err := file.Readdir(0)
if err != nil {
return "", fmt.Errorf("Unable to read from directory: %s", address)
return "", links, err
asdf commented 2019-11-17 20:07:20 +00:00
Outdated
Review
Copy Link

I've found it can be useful to include the error message coming from the FS operation. It will normally indicate why something has happened, and the error message is somewhat readable. There's other instances of this here, is this something you would consider changing?

I've found it can be useful to include the error message coming from the FS operation. It will normally indicate why something has happened, and the error message is somewhat readable. There's other instances of this here, is this something you would consider changing?
sloum commented 2019-11-17 22:59:44 +00:00
Review
Copy Link

I agree. The error messaging for this module is generally pretty readable and will be useful to users. I have changed this in the relevant locations in locals.go.

I agree. The error messaging for this module is generally pretty readable and will be useful to users. I have changed this in the relevant locations in `locals.go`.
}
var out strings.Builder
out.WriteString(fmt.Sprintf("Current directory: %s\n\n", address))
for _, obj := range fileList {
out.WriteString(obj)
out.WriteString("\n")
// Handle 'addres/..' display
asdf commented 2019-11-17 20:27:28 +00:00
Outdated
Review
Copy Link

Looking at bash, it includes ".." in the list of files in root, so this distinction may not be needed from a user perspective. The behaviour I see is that following .. from root just lands you back at root. Removing this logic check seems to do the same thing with no error messages, maybe it can be omitted?

If you did want to keep doing this, hardcoding root as "/" may not be as compatible as using filepath.VolumeName (but, I don't really know, it is just the closest thing I could see).

Looking at bash, it includes ".." in the list of files in root, so this distinction may not be needed from a user perspective. The behaviour I see is that following `..` from root just lands you back at root. Removing this logic check seems to do the same thing with no error messages, maybe it can be omitted? If you did want to keep doing this, hardcoding root as `"/"` may not be as compatible as using `filepath.VolumeName` (but, I don't really know, it is just the closest thing I could see).
sloum commented 2019-11-17 23:01:24 +00:00
Review
Copy Link

Yup, a quick test in play.golang.org shows that this works well. I agree ditching that hard-coded base path is a better cross-platform solution. I have removed the if and it just always builds the .. path version. I also added some basic comments to the code in-case it ever needs revisiting.

Yup, a quick test in `play.golang.org` shows that this works well. I agree ditching that hard-coded base path is a better cross-platform solution. I have removed the `if` and it just always builds the `..` path version. I also added some basic comments to the code in-case it ever needs revisiting.
offset = 2
upFp := filepath.Join(address, "..")
upOneLevel, _ := filepath.Abs(upFp)
info, err := os.Stat(upOneLevel)
if err == nil {
out.WriteString("[1] ")
out.WriteString(fmt.Sprintf("%-12s ", info.Mode().String()))
out.WriteString("../\n")
links = append(links, upOneLevel)
}
return out.String(), nil
// Sort the directory contents alphabetically
sort.Slice(fileList, func(i, j int) bool {
return fileList[i].Name() < fileList[j].Name()
})
// Handle each item in the directory
for i, obj := range fileList {
linkNum := fmt.Sprintf("[%d]", i+offset)
out.WriteString(fmt.Sprintf("%-5s ", linkNum))
out.WriteString(fmt.Sprintf("%-12s ", obj.Mode().String()))
out.WriteString(obj.Name())
if obj.IsDir() {
out.WriteString("/")
}
out.WriteString("\n")
fp := filepath.Join(address, obj.Name())
links = append(links, fp)
}
return out.String(), links, nil
}
bytes, err := ioutil.ReadAll(file)
if err != nil {
return "", fmt.Errorf("Unable to read file: %s", address)
return "", links, err
}
return string(bytes), nil
return string(bytes), links, nil
}
func pathExists(p string) bool {

2
page.go
View File

@ -70,7 +70,7 @@ func (p *Page) WrapContent(width int) {
content.WriteRune('\n')
counter = 0
}
} else if ch == '\r' || ch == '\v' || ch == '\b' || ch == '\f' {
} else if ch == '\r' || ch == '\v' || ch == '\b' || ch == '\f' || ch == '\a' {
// Get rid of control characters we dont want
continue
} else if ch == 27 {