From 214a15bc40061e3a653c4e194465bc57901d36e3 Mon Sep 17 00:00:00 2001 From: SmallCoccinelle <89733524+SmallCoccinelle@users.noreply.github.com> Date: Wed, 20 Oct 2021 07:10:46 +0200 Subject: [PATCH] Fixups + enable the commentFormatting linter (#1866) * Add a space after // comments For consistency, the commentFormatting lint checker suggests a space after each // comment block. This commit handles all the spots in the code where that is needed. * Rewrite documentation on functions Use the Go idiom of commenting: * First sentence declares the purpose. * First word is the name being declared The reason this style is preferred is such that grep is able to find names the user might be interested in. Consider e.g., go doc -all pkg/ffmpeg | grep -i transcode in which case a match will tell you the name of the function you are interested in. * Remove old code comment-blocks There are some commented out old code blocks in the code base. These are either 3 years old, or 2 years old. By now, I don't think their use is going to come back any time soon, and Git will track old pieces of deleted code anyway. Opt for deletion. * Reorder imports Split stdlib imports from non-stdlib imports in files we are touching. * Use a range over an iteration variable Probably more go-idiomatic, and the code needed comment-fixing anyway. * Use time.After rather than rolling our own The idiom here is common enough that the stdlib contains a function for it. Use the stdlib function over our own variant. * Enable the commentFormatting linter --- .golangci.yml | 2 -- pkg/api/check_version.go | 2 +- pkg/api/resolver.go | 2 +- pkg/ffmpeg/encoder_transcode.go | 10 ++++---- pkg/ffmpeg/ffprobe.go | 14 ++++------- pkg/ffmpeg/media_detection.go | 14 ++++++----- pkg/image/export_test.go | 2 +- pkg/logger/logger.go | 4 ---- pkg/manager/config/config.go | 4 ++-- pkg/manager/config/config_concurrency_test.go | 1 - pkg/manager/exclude_files.go | 11 ++++----- pkg/manager/exclude_files_test.go | 23 ++++++++++--------- pkg/manager/filename_parser.go | 5 ++-- pkg/manager/manager_tasks.go | 12 +++------- pkg/manager/task_transcode.go | 2 +- pkg/sqlite/repository.go | 4 ++-- pkg/utils/image.go | 7 ------ 17 files changed, 47 insertions(+), 72 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0499b6f29..ce6b3018e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -38,8 +38,6 @@ linters: linters-settings: gocritic: disabled-checks: - # Way too many errors to fix regarding comment formatting for now - - commentFormatting - appendAssign gofmt: diff --git a/pkg/api/check_version.go b/pkg/api/check_version.go index 5586b724d..e97aa3212 100644 --- a/pkg/api/check_version.go +++ b/pkg/api/check_version.go @@ -16,7 +16,7 @@ import ( "github.com/stashapp/stash/pkg/logger" ) -//we use the github REST V3 API as no login is required +// we use the github REST V3 API as no login is required const apiReleases string = "https://api.github.com/repos/stashapp/stash/releases" const apiTags string = "https://api.github.com/repos/stashapp/stash/tags" const apiAcceptHeader string = "application/vnd.github.v3+json" diff --git a/pkg/api/resolver.go b/pkg/api/resolver.go index 0cbcd2db2..2317f64e5 100644 --- a/pkg/api/resolver.go +++ b/pkg/api/resolver.go @@ -164,7 +164,7 @@ func (r *queryResolver) Version(ctx context.Context) (*models.Version, error) { }, nil } -//Gets latest version (git shorthash commit for now) +// Latestversion returns the latest git shorthash commit. func (r *queryResolver) Latestversion(ctx context.Context) (*models.ShortVersion, error) { ver, url, err := GetLatestVersion(ctx, true) if err == nil { diff --git a/pkg/ffmpeg/encoder_transcode.go b/pkg/ffmpeg/encoder_transcode.go index 235fb6959..920051b96 100644 --- a/pkg/ffmpeg/encoder_transcode.go +++ b/pkg/ffmpeg/encoder_transcode.go @@ -67,9 +67,9 @@ func (e *Encoder) Transcode(probeResult VideoFile, options TranscodeOptions) { _, _ = e.runTranscode(probeResult, args) } -//transcode the video, remove the audio -//in some videos where the audio codec is not supported by ffmpeg -//ffmpeg fails if you try to transcode the audio +// TranscodeVideo transcodes the video, and removes the audio. +// In some videos where the audio codec is not supported by ffmpeg, +// ffmpeg fails if you try to transcode the audio func (e *Encoder) TranscodeVideo(probeResult VideoFile, options TranscodeOptions) { scale := calculateTranscodeScale(probeResult, options.MaxTranscodeSize) args := []string{ @@ -87,7 +87,7 @@ func (e *Encoder) TranscodeVideo(probeResult VideoFile, options TranscodeOptions _, _ = e.runTranscode(probeResult, args) } -//copy the video stream as is, transcode audio +// TranscodeAudio will copy the video stream as is, and transcode audio. func (e *Encoder) TranscodeAudio(probeResult VideoFile, options TranscodeOptions) { args := []string{ "-i", probeResult.Path, @@ -99,7 +99,7 @@ func (e *Encoder) TranscodeAudio(probeResult VideoFile, options TranscodeOptions _, _ = e.runTranscode(probeResult, args) } -//copy the video stream as is, drop audio +// CopyVideo will copy the video stream as is, and drop the audio stream. func (e *Encoder) CopyVideo(probeResult VideoFile, options TranscodeOptions) { args := []string{ "-i", probeResult.Path, diff --git a/pkg/ffmpeg/ffprobe.go b/pkg/ffmpeg/ffprobe.go index e97db8aaf..dfde16d8b 100644 --- a/pkg/ffmpeg/ffprobe.go +++ b/pkg/ffmpeg/ffprobe.go @@ -72,8 +72,8 @@ var validAudioForMkv = []AudioCodec{Aac, Mp3, Vorbis, Opus} var validAudioForWebm = []AudioCodec{Vorbis, Opus} var validAudioForMp4 = []AudioCodec{Aac, Mp3} -//maps user readable container strings to ffprobe's format_name -//on some formats ffprobe can't differentiate +// ContainerToFfprobe maps user readable container strings to ffprobe's format_name. +// On some formats ffprobe can't differentiate var ContainerToFfprobe = map[Container]string{ Mp4: Mp4Ffmpeg, M4v: M4vFfmpeg, @@ -155,7 +155,8 @@ func IsValidForContainer(format Container, validContainers []Container) bool { return false } -//extend stream validation check to take into account container +// IsValidCombo checks if a codec/container combination is valid. +// Returns true on validity, false otherwise func IsValidCombo(codecName string, format Container, supportedVideoCodecs []string) bool { supportMKV := IsValidCodec(Mkv, supportedVideoCodecs) supportHEVC := IsValidCodec(Hevc, supportedVideoCodecs) @@ -227,10 +228,6 @@ type FFProbe string // Execute exec command and bind result to struct. func (f *FFProbe) NewVideoFile(videoPath string, stripExt bool) (*VideoFile, error) { args := []string{"-v", "quiet", "-print_format", "json", "-show_format", "-show_streams", "-show_error", videoPath} - //// Extremely slow on windows for some reason - //if runtime.GOOS != "windows" { - // args = append(args, "-count_frames") - //} out, err := exec.Command(string(*f), args...).Output() if err != nil { @@ -256,9 +253,6 @@ func parse(filePath string, probeJSON *FFProbeJSON, stripExt bool) (*VideoFile, if result.JSON.Error.Code != 0 { return nil, fmt.Errorf("ffprobe error code %d: %s", result.JSON.Error.Code, result.JSON.Error.String) } - //} else if (ffprobeResult.stderr.includes("could not find codec parameters")) { - // throw new Error(`FFProbe [${filePath}] -> Could not find codec parameters`); - //} // TODO nil_or_unsupported.(video_stream) && nil_or_unsupported.(audio_stream) result.Path = filePath result.Title = probeJSON.Format.Tags.Title diff --git a/pkg/ffmpeg/media_detection.go b/pkg/ffmpeg/media_detection.go index 4de7e4ba6..6d9feb59a 100644 --- a/pkg/ffmpeg/media_detection.go +++ b/pkg/ffmpeg/media_detection.go @@ -2,8 +2,9 @@ package ffmpeg import ( "bytes" - "github.com/stashapp/stash/pkg/logger" "os" + + "github.com/stashapp/stash/pkg/logger" ) // detect file format from magic file number @@ -37,11 +38,12 @@ func containsMatroskaSignature(buf, subType []byte) bool { return buf[index-3] == 0x42 && buf[index-2] == 0x82 } -//returns container as string ("" on error or no match) -//implements only mkv or webm as ffprobe can't distinguish between them -//and not all browsers support mkv -func MagicContainer(file_path string) Container { - file, err := os.Open(file_path) +// MagicContainer returns the container type of a file path. +// Returns the zero-value on errors or no-match. Implements mkv or +// webm only, as ffprobe can't distinguish between them and not all +// browsers support mkv +func MagicContainer(filePath string) Container { + file, err := os.Open(filePath) if err != nil { logger.Errorf("[magicfile] %v", err) return "" diff --git a/pkg/image/export_test.go b/pkg/image/export_test.go index 6f72ea324..f47672b58 100644 --- a/pkg/image/export_test.go +++ b/pkg/image/export_test.go @@ -50,7 +50,7 @@ const ( const ( studioName = "studioName" - //galleryChecksum = "galleryChecksum" + // galleryChecksum = "galleryChecksum" ) var ( diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 96d4eba3e..0b4a4225b 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -295,7 +295,3 @@ func Fatal(args ...interface{}) { func Fatalf(format string, args ...interface{}) { logger.Fatalf(format, args...) } - -//func WithRequest(req *http.Request) *logrus.Entry { -// return logger.WithFields(RequestFields(req)) -//} diff --git a/pkg/manager/config/config.go b/pkg/manager/config/config.go index 7721413ad..be63a4c0d 100644 --- a/pkg/manager/config/config.go +++ b/pkg/manager/config/config.go @@ -9,7 +9,7 @@ import ( "strings" "sync" - //"github.com/sasha-s/go-deadlock" // if you have deadlock issues + // "github.com/sasha-s/go-deadlock" // if you have deadlock issues "golang.org/x/crypto/bcrypt" @@ -190,7 +190,7 @@ type Instance struct { certFile string keyFile string sync.RWMutex - //deadlock.RWMutex // for deadlock testing/issues + // deadlock.RWMutex // for deadlock testing/issues } var instance *Instance diff --git a/pkg/manager/config/config_concurrency_test.go b/pkg/manager/config/config_concurrency_test.go index cc6b56d67..efac016ab 100644 --- a/pkg/manager/config/config_concurrency_test.go +++ b/pkg/manager/config/config_concurrency_test.go @@ -10,7 +10,6 @@ func TestConcurrentConfigAccess(t *testing.T) { i := GetInstance() const workers = 8 - //const loops = 1000 const loops = 200 var wg sync.WaitGroup for k := 0; k < workers; k++ { diff --git a/pkg/manager/exclude_files.go b/pkg/manager/exclude_files.go index 333a3b151..b80b2f911 100644 --- a/pkg/manager/exclude_files.go +++ b/pkg/manager/exclude_files.go @@ -22,14 +22,13 @@ func excludeFiles(files []string, patterns []string) ([]string, int) { return files, 0 } - for i := 0; i < len(files); i++ { - if matchFileSimple(files[i], fileRegexps) { - logger.Infof("File matched pattern. Excluding:\"%s\"", files[i]) + for _, f := range files { + if matchFileSimple(f, fileRegexps) { + logger.Infof("File matched pattern. Excluding:\"%s\"", f) exclCount++ } else { - - //if pattern doesn't match add file to list - results = append(results, files[i]) + // if pattern doesn't match add file to list + results = append(results, f) } } logger.Infof("Excluded %d file(s) from scan", exclCount) diff --git a/pkg/manager/exclude_files_test.go b/pkg/manager/exclude_files_test.go index 978c9455e..df8a0a140 100644 --- a/pkg/manager/exclude_files_test.go +++ b/pkg/manager/exclude_files_test.go @@ -2,8 +2,9 @@ package manager import ( "fmt" - "github.com/stashapp/stash/pkg/logger" "testing" + + "github.com/stashapp/stash/pkg/logger" ) var excludeTestFilenames = []string{ @@ -31,16 +32,16 @@ var excludeTests = []struct { testPattern []string expected int }{ - {[]string{"sample\\.mp4$", "trash", "\\.[\\d]{3}\\.webm$"}, 6}, //generic - {[]string{"no_match\\.mp4"}, 0}, //no match - {[]string{"^/stash/videos/exclude/", "/videos/xcl/"}, 3}, //linux - {[]string{"/\\.[[:word:]]+/"}, 1}, //linux hidden dirs (handbrake unraid issue?) - {[]string{"c:\\\\stash\\\\videos\\\\exclude"}, 1}, //windows - {[]string{"\\/[/invalid"}, 0}, //invalid pattern - {[]string{"\\/[/invalid", "sample\\.[[:alnum:]]+$"}, 3}, //invalid pattern but continue - {[]string{"^\\\\\\\\network"}, 4}, //windows net share - {[]string{"\\\\private\\\\"}, 1}, //windows net share - {[]string{"\\\\private\\\\", "sample\\.mp4"}, 3}, //windows net share + {[]string{"sample\\.mp4$", "trash", "\\.[\\d]{3}\\.webm$"}, 6}, // generic + {[]string{"no_match\\.mp4"}, 0}, // no match + {[]string{"^/stash/videos/exclude/", "/videos/xcl/"}, 3}, // linux + {[]string{"/\\.[[:word:]]+/"}, 1}, // linux hidden dirs (handbrake unraid issue?) + {[]string{"c:\\\\stash\\\\videos\\\\exclude"}, 1}, // windows + {[]string{"\\/[/invalid"}, 0}, // invalid pattern + {[]string{"\\/[/invalid", "sample\\.[[:alnum:]]+$"}, 3}, // invalid pattern but continue + {[]string{"^\\\\\\\\network"}, 4}, // windows net share + {[]string{"\\\\private\\\\"}, 1}, // windows net share + {[]string{"\\\\private\\\\", "sample\\.mp4"}, 3}, // windows net share } func TestExcludeFiles(t *testing.T) { diff --git a/pkg/manager/filename_parser.go b/pkg/manager/filename_parser.go index 9dbfecd81..def35837c 100644 --- a/pkg/manager/filename_parser.go +++ b/pkg/manager/filename_parser.go @@ -3,13 +3,14 @@ package manager import ( "database/sql" "errors" - "github.com/stashapp/stash/pkg/studio" "path/filepath" "regexp" "strconv" "strings" "time" + "github.com/stashapp/stash/pkg/studio" + "github.com/stashapp/stash/pkg/models" "github.com/stashapp/stash/pkg/tag" ) @@ -81,8 +82,6 @@ func initParserFields() { ret["title"] = newParserField("title", ".*", true) ret["ext"] = newParserField("ext", ".*$", false) - //I = new ParserField("i", undefined, "Matches any ignored word", false); - ret["d"] = newParserField("d", `(?:\.|-|_)`, false) ret["rating"] = newParserField("rating", `\d`, true) ret["performer"] = newParserField("performer", ".*", true) diff --git a/pkg/manager/manager_tasks.go b/pkg/manager/manager_tasks.go index fbffa5b49..203b76ec3 100644 --- a/pkg/manager/manager_tasks.go +++ b/pkg/manager/manager_tasks.go @@ -514,14 +514,8 @@ func (s *singleton) neededGenerate(scenes []*models.Scene, input models.Generate var totals totalsGenerate const timeout = 90 * time.Second - // create a control channel through which to signal the counting loop when the timeout is reached - chTimeout := make(chan struct{}) - - //run the timeout function in a separate thread - go func() { - time.Sleep(timeout) - chTimeout <- struct{}{} - }() + // Set a deadline. + chTimeout := time.After(timeout) fileNamingAlgo := config.GetInstance().GetVideoFileNamingAlgorithm() overwrite := false @@ -592,7 +586,7 @@ func (s *singleton) neededGenerate(scenes []*models.Scene, input models.Generate } } } - //check for timeout + // check for timeout select { case <-chTimeout: return nil diff --git a/pkg/manager/task_transcode.go b/pkg/manager/task_transcode.go index 807f1c2e3..1061852fb 100644 --- a/pkg/manager/task_transcode.go +++ b/pkg/manager/task_transcode.go @@ -69,7 +69,7 @@ func (t *GenerateTranscodeTask) Start() { } } else { if audioCodec == ffmpeg.MissingUnsupported { - //ffmpeg fails if it trys to transcode an unsupported audio codec + // ffmpeg fails if it trys to transcode an unsupported audio codec encoder.TranscodeVideo(*videoFile, options) } else { encoder.Transcode(*videoFile, options) diff --git a/pkg/sqlite/repository.go b/pkg/sqlite/repository.go index c9296fe54..1254afa06 100644 --- a/pkg/sqlite/repository.go +++ b/pkg/sqlite/repository.go @@ -432,7 +432,7 @@ func listKeys(i interface{}, addPrefix bool) string { var query []string v := reflect.ValueOf(i) for i := 0; i < v.NumField(); i++ { - //get key for struct tag + // Get key for struct tag rawKey := v.Type().Field(i).Tag.Get("db") key := strings.Split(rawKey, ",")[0] if key == "id" { @@ -450,7 +450,7 @@ func updateSet(i interface{}, partial bool) string { var query []string v := reflect.ValueOf(i) for i := 0; i < v.NumField(); i++ { - //get key for struct tag + // Get key for struct tag rawKey := v.Type().Field(i).Tag.Get("db") key := strings.Split(rawKey, ",")[0] if key == "id" { diff --git a/pkg/utils/image.go b/pkg/utils/image.go index d1993af98..59435160f 100644 --- a/pkg/utils/image.go +++ b/pkg/utils/image.go @@ -106,13 +106,6 @@ func GetDataFromBase64String(encodedString string) ([]byte, error) { // GetBase64StringFromData returns the given byte slice as a base64 encoded string func GetBase64StringFromData(data []byte) string { return base64.StdEncoding.EncodeToString(data) - - // Really slow - //result = regexp.MustCompile(`(.{60})`).ReplaceAllString(result, "$1\n") - //if result[len(result)-1:] != "\n" { - // result += "\n" - //} - //return result } func ServeImage(image []byte, w http.ResponseWriter, r *http.Request) error {