-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add hints for failing conditions #172
Changes from 1 commit
c4965eb
3344ae9
2fafebb
1d29e54
c688cdf
3795a0d
cfb8cdc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ type imageData struct { | |
TotalPoints int | ||
Penalties []scoreItem | ||
Points []scoreItem | ||
Hints []hintItem | ||
} | ||
|
||
// connData represents the current connectivity state of the image to the | ||
|
@@ -46,10 +47,19 @@ type connData struct { | |
// scoreItem is the scoring report representation of a check, containing only | ||
// the message and points associated with it. | ||
type scoreItem struct { | ||
Index int | ||
Message string | ||
Points int | ||
} | ||
|
||
// hintItem is the scoring report representation of a hint, which can contain | ||
// multiple messages. | ||
type hintItem struct { | ||
Index int | ||
Messages []string | ||
Points int | ||
} | ||
|
||
// config is a representation of the TOML configuration typically | ||
// specific in scoring.conf. | ||
type config struct { | ||
|
@@ -209,19 +219,25 @@ func scoreCheck(check check) { | |
status := false | ||
failed := false | ||
|
||
// If a fail condition passes, the check fails, no other checks required. | ||
// Create hint var in case any checks have hints | ||
hint := hintItem{ | ||
Index: checkCount, | ||
Points: check.Points, | ||
} | ||
|
||
// If a Fail condition passes, the check fails, no other checks required. | ||
if len(check.Fail) > 0 { | ||
failed = checkFails(&check) | ||
failed = checkOr(&check, &hint) | ||
} | ||
|
||
// If a PassOverride succeeds, that overrides the Pass checks | ||
if !failed && len(check.PassOverride) > 0 { | ||
status = checkPassOverrides(&check) | ||
status = checkOr(&check, &hint) | ||
} | ||
|
||
// Finally, we check the normal pass checks | ||
// Finally, we check the normal Pass checks | ||
if !failed && !status && len(check.Pass) > 0 { | ||
status = checkPass(&check) | ||
status = checkAnd(&check, &hint) | ||
} | ||
|
||
if status { | ||
|
@@ -231,18 +247,24 @@ func scoreCheck(check check) { | |
pass(fmt.Sprintf("Check passed: %s - %d pts", check.Message, check.Points)) | ||
obfuscateData(&check.Message) | ||
} | ||
image.Points = append(image.Points, scoreItem{check.Message, check.Points}) | ||
image.Points = append(image.Points, scoreItem{checkCount, check.Message, check.Points}) | ||
image.Contribs += check.Points | ||
} else { | ||
if verboseEnabled { | ||
deobfuscateData(&check.Message) | ||
fail(fmt.Sprintf("Penalty triggered: %s - %d pts", check.Message, check.Points)) | ||
obfuscateData(&check.Message) | ||
} | ||
image.Penalties = append(image.Penalties, scoreItem{check.Message, check.Points}) | ||
image.Penalties = append(image.Penalties, scoreItem{checkCount, check.Message, check.Points}) | ||
image.Detracts += check.Points | ||
} | ||
image.Score += check.Points | ||
} else { | ||
// If the check failed, and there are hints, see if we should display them. | ||
// All hints triggered (based on which conditions ran) are displayed in sequential order. | ||
if len(hint.Messages) > 0 { | ||
image.Hints = append(image.Hints, hint) | ||
} | ||
} | ||
|
||
// If check is not a penalty, add to total | ||
|
@@ -252,27 +274,24 @@ func scoreCheck(check check) { | |
} | ||
} | ||
|
||
func checkFails(check *check) bool { | ||
func checkOr(check *check, hint *hintItem) bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we have a line or two to explain what There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw that the functions were identical for checkFails/PassOverride because they were both OR/|| behavior, in that on the first true, return. Whereas pass is AND/&& because everything needs to be true otherwise return false. Sure I'll add a comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually good catch I messed that up and forgot to change it to take a list of conds rather than a check |
||
for _, cond := range check.Fail { | ||
if runCheck(cond) { | ||
return true | ||
} | ||
} | ||
return false | ||
} | ||
|
||
func checkPassOverrides(check *check) bool { | ||
for _, cond := range check.PassOverride { | ||
if runCheck(cond) { | ||
return true | ||
if cond.Hint != "" { | ||
hint.Messages = append(hint.Messages, cond.Hint) | ||
} | ||
} | ||
return false | ||
} | ||
|
||
func checkPass(check *check) bool { | ||
func checkAnd(check *check, hint *hintItem) bool { | ||
for _, cond := range check.Pass { | ||
if !runCheck(cond) { | ||
if cond.Hint != "" { | ||
hint.Messages = append(hint.Messages, cond.Hint) | ||
} | ||
return false | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it was
Printf()
before, this is just a sanity check, but it is intentional that it isPrint()
and notPrintln()
yeah?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sorry had this change lying around for a while, it is supposed to be Print. Printf was kind of like a format string vulnerability lol, not that those exist in go. The newline is added in toPrint via Sprintln.