Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor align converter #623

Merged
merged 1 commit into from
Sep 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ require (
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.19.0
github.com/ulikunitz/xz v0.5.12
golang.org/x/exp v0.0.0-20240808152545-0cdaa3abc0fa
golang.org/x/exp v0.0.0-20240904232852-e7e105dedf7e
golang.org/x/sync v0.8.0
golang.org/x/term v0.23.0
)
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/exp v0.0.0-20240808152545-0cdaa3abc0fa h1:ELnwvuAXPNtPk1TJRuGkI9fDTwym6AYBu0qzT8AcHdI=
golang.org/x/exp v0.0.0-20240808152545-0cdaa3abc0fa/go.mod h1:akd2r19cwCdwSwWeIdzYQGa/EZZyqcOdwWiwj5L5eKQ=
golang.org/x/exp v0.0.0-20240904232852-e7e105dedf7e h1:I88y4caeGeuDQxgdoFPUq097j7kNfw6uvuiNxUBfcBk=
golang.org/x/exp v0.0.0-20240904232852-e7e105dedf7e/go.mod h1:akd2r19cwCdwSwWeIdzYQGa/EZZyqcOdwWiwj5L5eKQ=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
Expand Down
32 changes: 16 additions & 16 deletions oviewer/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ func (root *Root) goLineNumber(lN int) {
root.setMessagef("Moved to line %d", lN+1)
}

// markNext moves to the next mark.
func (root *Root) markNext(context.Context) {
// nextMark moves to the next mark.
func (root *Root) nextMark(context.Context) {
Comment on lines -247 to +248
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't rename them in the same logic as

#623 (comment)

But why not

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depends on the naming rules of the function name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I said, why not 😁

if len(root.Doc.marked) == 0 {
return
}
Expand All @@ -258,8 +258,8 @@ func (root *Root) markNext(context.Context) {
root.goLineNumber(root.Doc.marked[root.Doc.markedPoint])
}

// markPrev moves to the previous mark.
func (root *Root) markPrev(context.Context) {
// prevMark moves to the previous mark.
func (root *Root) prevMark(context.Context) {
if len(root.Doc.marked) == 0 {
return
}
Expand Down Expand Up @@ -410,22 +410,22 @@ func (root *Root) setViewMode(ctx context.Context, modeName string) {
root.setMessage(err.Error())
return
}

root.Doc.general = mergeGeneral(root.Doc.general, c)
root.Doc.conv = root.Doc.converterType(root.Doc.general.Converter)
root.Doc.regexpCompile()
root.Doc.ClearCache()
m := root.Doc
m.general = mergeGeneral(m.general, c)
m.conv = m.converterType(m.general.Converter)
m.regexpCompile()
m.ClearCache()
root.ViewSync(ctx)
// Set caption.
if root.Doc.general.Caption != "" {
root.Doc.Caption = root.Doc.general.Caption
if m.general.Caption != "" {
m.Caption = m.general.Caption
}
root.setMessagef("Set mode %s", modeName)
}

// modeConfig returns the configuration of the specified mode.
func (root *Root) modeConfig(modeName string) (general, error) {
if modeName == "general" {
if modeName == generalName {
return root.General, nil
}

Expand All @@ -444,13 +444,13 @@ func (root *Root) setConverter(ctx context.Context, name string) {
}
m.general.Converter = name
m.conv = m.converterType(name)
root.Doc.ClearCache()
m.ClearCache()
root.ViewSync(ctx)
}

// alignFormat sets converter type to align.
func (root *Root) alignFormat(ctx context.Context) {
if root.Doc.Converter == "align" {
if root.Doc.Converter == alignConv {
root.esFormat(ctx)
return
}
Expand All @@ -460,7 +460,7 @@ func (root *Root) alignFormat(ctx context.Context) {

// rawFormat sets converter type to raw.
func (root *Root) rawFormat(ctx context.Context) {
if root.Doc.Converter == "raw" {
if root.Doc.Converter == rawConv {
root.esFormat(ctx)
return
}
Expand Down Expand Up @@ -777,7 +777,7 @@ func (root *Root) shrinkColumnToggle(ctx context.Context) {
// ShrinkColumn shrinks or expands the specified column.
func (root *Root) ShrinkColumn(ctx context.Context, cursor int) bool {
m := root.Doc
if root.Doc.Converter != "align" {
if root.Doc.Converter != alignConv {
root.setMessage("Not align mode")
return false
}
Expand Down
20 changes: 10 additions & 10 deletions oviewer/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ func TestRoot_Mark(t *testing.T) {
})
}

func TestRoot_markNext(t *testing.T) {
func TestRoot_nextMark(t *testing.T) {
tcellNewScreen = fakeScreen
defer func() {
tcellNewScreen = tcell.NewScreen
Expand Down Expand Up @@ -1126,18 +1126,18 @@ func TestRoot_markNext(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root.markNext(context.Background()) // no marked
root.nextMark(context.Background()) // no marked
root.Doc.marked = []int{1, 3, 5}
root.Doc.markedPoint = tt.markedPoint
root.markNext(context.Background())
root.nextMark(context.Background())
if root.Doc.topLN != tt.wantLine {
t.Errorf("got line %d, want line %d", root.Doc.topLN, tt.wantLine)
}
})
}
}

func TestRoot_markPrev(t *testing.T) {
func TestRoot_prevMark(t *testing.T) {
tcellNewScreen = fakeScreen
defer func() {
tcellNewScreen = tcell.NewScreen
Expand Down Expand Up @@ -1168,10 +1168,10 @@ func TestRoot_markPrev(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root.markPrev(context.Background()) // no marked
root.prevMark(context.Background()) // no marked
root.Doc.marked = []int{1, 3, 5}
root.Doc.markedPoint = tt.markedPoint
root.markPrev(context.Background())
root.prevMark(context.Background())
if root.Doc.topLN != tt.wantLine {
t.Errorf("got line %d, want line %d", root.Doc.topLN, tt.wantLine)
}
Expand Down Expand Up @@ -1250,7 +1250,7 @@ func TestRoot_modeConfig(t *testing.T) {
{
name: "testModeConfig general",
args: args{
modeName: "general",
modeName: generalName,
},
want: general{},
wantErr: false,
Expand Down Expand Up @@ -1527,21 +1527,21 @@ func TestRoot_setConverter(t *testing.T) {
{
name: "testSetConverterEscape",
args: args{
name: "es",
name: esConv,
},
want: newESConverter(),
},
{
name: "testSetConverterRaw",
args: args{
name: "raw",
name: rawConv,
},
want: newRawConverter(),
},
{
name: "testSetConverterAlign",
args: args{
name: "align",
name: alignConv,
},
want: newAlignConverter(false),
},
Expand Down
15 changes: 9 additions & 6 deletions oviewer/convert_align.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import (
// It is used to align columns when the delimiter is reached or to align columns by adding spaces to the end of the line.
type align struct {
es *escapeSequence
maxWidths []int // column max width
maxWidths []int // Maximum width of each column.
orgWidths []int
shrink []bool
rightAlign []bool
shrink []bool // Shrink column.
rightAlign []bool // Right align column.
WidthF bool
delimiter string
delimiterReg *regexp.Regexp
Expand All @@ -22,9 +22,12 @@ type align struct {

func newAlignConverter(widthF bool) *align {
return &align{
es: newESConverter(),
count: 0,
WidthF: widthF,
es: newESConverter(),
maxWidths: []int{},
shrink: []bool{},
rightAlign: []bool{},
count: 0,
WidthF: widthF,
}
}

Expand Down
2 changes: 1 addition & 1 deletion oviewer/input_viewmode.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (root *Root) setViewInputMode(context.Context) {
func viewModeCandidate() *candidate {
return &candidate{
list: []string{
"general",
generalName,
},
}
}
Expand Down
4 changes: 2 additions & 2 deletions oviewer/keybind.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ func (root *Root) handlers() map[string]func(context.Context) {
actionNextSection: root.nextSection,
actionPrevSection: root.prevSection,
actionLastSection: root.lastSection,
actionMoveMark: root.markNext,
actionMovePrevMark: root.markPrev,
actionMoveMark: root.nextMark,
actionMovePrevMark: root.prevMark,
actionViewMode: root.setViewInputMode,
actionWrap: root.toggleWrapMode,
actionColumnMode: root.toggleColumnMode,
Expand Down
13 changes: 9 additions & 4 deletions oviewer/oviewer.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,15 @@ const QuitSmallCountDown = 10
// when the debug flag is enabled.
const MaxWriteLog int = 10

// The name of the converter that can be specified.
const (
esConv = "es"
rawConv = "raw"
alignConv = "align"
esConv string = "es" // esConv processes escape sequence(default).
rawConv string = "raw" // rawConv is displayed without processing escape sequences as they are.
alignConv string = "align" // alignConv is aligned in each column.
)

const (
generalName string = "general"
Comment on lines +365 to +371
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually constants are sharing a common prefix, allowing to find the easily when using completion in the IDE, they are also ordered in the godoc if they are exported

Suggested change
esConv string = "es" // esConv processes escape sequence(default).
rawConv string = "raw" // rawConv is displayed without processing escape sequences as they are.
alignConv string = "align" // alignConv is aligned in each column.
)
const (
generalName string = "general"
convEscaped string = "es" // convEscaped processes escape sequence(default).
convRaw string = "raw" // convRaw is displayed without processing escape sequences as they are.
convAlign string = "align" // convAlign is aligned in each column.
)
const (
nameGeneral string = "general"

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your comment.
That's right. Change.

)

var Shrink rune = '…'
Expand Down Expand Up @@ -729,7 +734,7 @@ func (root *Root) setCaption() {
// setViewModeConfig sets view mode config.
func (root *Root) setViewModeConfig() {
list := make([]string, 0, len(root.Config.Mode)+1)
list = append(list, "general")
list = append(list, generalName)
for name := range root.Config.Mode {
list = append(list, name)
}
Expand Down
2 changes: 1 addition & 1 deletion oviewer/oviewer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ func TestRoot_setViewModeConfig(t *testing.T) {
"view1": {},
},
},
wantList: []string{"general", "view1"},
wantList: []string{generalName, "view1"},
},
}
for _, tt := range tests {
Expand Down
25 changes: 11 additions & 14 deletions oviewer/prepare_draw.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"errors"
"log"
"math"
"reflect"
"regexp"
"slices"
"sort"
"strconv"
"time"
Expand Down Expand Up @@ -120,18 +120,19 @@ func (root *Root) setAlignConverter() {
maxWidths, addRight = m.maxColumnWidths(maxWidths, addRight, ln)
}

if !reflect.DeepEqual(m.alignConv.maxWidths, maxWidths) {
if !slices.Equal(m.alignConv.maxWidths, maxWidths) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based of the fact the if starts here and ends with the function, you could do this

Suggested change
if !slices.Equal(m.alignConv.maxWidths, maxWidths) {
if slices.Equal(m.alignConv.maxWidths, maxWidths) {
return
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, that's right.

m.alignConv.orgWidths = m.columnWidths
m.alignConv.maxWidths = maxWidths
m.alignConv.rightAlign = make([]bool, len(maxWidths)+1)
for n := len(m.alignConv.shrink); n < len(maxWidths)+1; n++ {
m.alignConv.shrink = append(m.alignConv.shrink, false)
}
for n := len(m.alignConv.shrink); n < len(maxWidths)+1; n++ {
for n := len(m.alignConv.rightAlign); n < len(maxWidths)+1; n++ {
m.alignConv.rightAlign = append(m.alignConv.rightAlign, false)
}
for i := 0; i < len(addRight); i++ {
m.alignConv.rightAlign[i] = addRight[i] > 1
if !m.alignConv.rightAlign[i] {
m.alignConv.rightAlign[i] = addRight[i] > 1
}
}
m.ClearCache()
}
Expand Down Expand Up @@ -200,37 +201,33 @@ func maxWidthsWidth(lc contents, maxWidths []int, widths []int, rightCount []int
return maxWidths, rightCount
}

// trimWidth returns the column width and 1 if the column is right-aligned.
func trimWidth(lc contents) (int, int) {
l := trimLeft(lc)
r := trimRight(lc)

l, r := trimmedIndices(lc)
addRight := 0
if l > len(lc)-r {
addRight = 1
}
return r - l, addRight
}

func trimLeft(lc contents) int {
// trimmedIndices returns the number of left spaces.
func trimmedIndices(lc contents) (int, int) {
ts := 0
for i := 0; i < len(lc); i++ {
if lc[i].mainc != ' ' {
ts = i
break
}
}
return ts
}
func trimRight(lc contents) int {
te := len(lc)
for i := len(lc) - 1; i >= 0; i-- {
if lc[i].mainc != ' ' {
te = i + 1
break
}
}
return te

return ts, te
}

// shiftBody shifts the section header so that it is not hidden by it.
Expand Down
2 changes: 1 addition & 1 deletion oviewer/prepare_draw_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ func TestRoot_setAlignConverter(t *testing.T) {
root.Doc.alignConv = newAlignConverter(true)
root.prepareDraw(ctx)
if got := root.Doc.alignConv.maxWidths; !reflect.DeepEqual(got, tt.want) {
t.Errorf("Root.setAlignConverter() = %v, want %v", got, tt.want)
t.Errorf("Root.setAlignConverter() = %#v, want %#v", got, tt.want)
}
})
}
Expand Down