PR#25: Always install SFiles whatever the arch - golist

golist

#25 Always install SFiles whatever the arch

Merged 4 years ago by qulogic. Opened 4 years ago by eclipseo.

eclipseo/golist add_extra_SFiles into master

Always install SFiles whatever the arch

Robert-André Mauchin • 4 years ago

b679c33

cmd/golist/golist.go

file modified

		`@@ -78,6 +78,7 @@`
		`cli.StringSliceFlag{`
		`Name: "include-extension, e",`
		`Usage: "Include all files with the extension in the recognized resources, e.g. .proto, .tmpl",`
		`+ Value: &cli.StringSlice{".proto", ".md", ".s"},`
		`},`
		`cli.BoolFlag{`
		`Name: "json",`

pkg/util/util.go

file modified

		`@@ -78,6 +78,7 @@`
		`ProtoFiles []string`
		`TmplFiles []string`
		`MDFiles []string`
		`+ SFiles []string`
		`Other []string`
		`}`

		`@@ -154,6 +155,8 @@`
		`p.otherResources.ProtoFiles = append(p.otherResources.ProtoFiles, path)`
		`case ".md":`
		`p.otherResources.MDFiles = append(p.otherResources.MDFiles, path)`
		`+ case ".s":`
		`+ p.otherResources.SFiles = append(p.otherResources.SFiles, path)`
		`default:`
		`p.otherResources.Other = append(p.otherResources.Other, path)`
		`}`
		`@@ -278,6 +281,9 @@`
		`if p.otherResources.MDFiles != nil {`
		`resources = append(resources, p.otherResources.MDFiles...)`
		`}`
		`+ if p.otherResources.SFiles != nil {`
		`+ resources = append(resources, p.otherResources.SFiles...)`
		`+ }`
		`if p.otherResources.Other != nil {`
		`resources = append(resources, p.otherResources.Other...)`
		`}`

eclipseo commented 4 years ago

Signed-off-by: Robert-André Mauchin zebob.m@gmail.com

nim commented 4 years ago

That looks good to me (I assume you checked it builds and works). That's how I would have done it.

@qulogic up to you now

eclipseo commented 4 years ago

I assume you checked it builds and works

I haven't. But I assume it would work.

eclipseo commented 4 years ago

Tested and it doesn't work.

eclipseo commented 4 years ago

It seems the "other files" are not included by default but need to be specified with --include-extension.
I don't understand the point of the switch case with .proto, .md and so on thoughk

Edited 4 years ago by eclipseo

nim commented 4 years ago

Ok, weird that @jchaloup gone to all this pain for something that does not work

Reading the urfave/cli documentation (that I don’t know well, I used jawher/mow.cli in modist), it seems you need to define a default extension value

cli.NewStringSlice(".s", ".proto")

(https://github.com/urfave/cli/pull/392)

and then set it as value

Value: cli.NewStringSlice(".s", ".proto")

here https://pagure.io/golist/blob/master/f/cmd/golist/golist.go#_80

But, I may be misreading the documentation, and I have no access to by coding setup right now :( It needs testing

rebased onto b679c33

4 years ago

eclipseo commented 4 years ago

Used:

            Value: &cli.StringSlice{".proto", ".md", ".s"},

We're still using the v1 of urfave/cli. v2 never caught up and the whole project seems abandoned since 2016.

Tested in COPR and it works: https://copr.fedorainfracloud.org/coprs/eclipseo/golang-ng/build/935905/

qulogic commented 4 years ago

LGTM.

Pull-Request has been merged by qulogic

4 years ago

Metadata

Assignee

None

Tags

No Tags

Changes Summary 2

golist

Source Code

#25 Always install SFiles whatever the arch Merged 4 years ago by qulogic. Opened 4 years ago by eclipseo. eclipseo/golist add_extra_SFiles into master

Metadata

Changes Summary 2

#25 Always install SFiles whatever the arch

Merged 4 years ago by qulogic. Opened 4 years ago by eclipseo.

eclipseo/golist add_extra_SFiles into master