[refactor] Type-driven refactoring: remove span_id in PrefixPrint() #1491
Conversation
The same can be done for Print_() and PrintMessage(), which now take a span_id, but should take a location.
There was a problem hiding this comment.
I added a few comments which I think will help with similar changes
Also notice that we are mostly getting rid of keyword arguments, which means getting rid of names at the call site. I think it's OK to have 3 args in any function ... the type system helps prevent errors.
| @@ -139,7 +139,7 @@ def RunBuiltin(self, builtin_id, cmd_val): | |||
| if e.span_id == runtime.NO_SPID: | |||
| e.span_id = self.errfmt.CurrentLocation() | |||
| # e.g. 'type' doesn't accept flag '-x' | |||
| self.errfmt.PrefixPrint(e.msg, prefix='%r ' % arg0, span_id=e.span_id) | |||
| self.errfmt.PrefixPrint(e.msg, '%r ' % arg0, loc.Span(e.span_id)) | |||
There was a problem hiding this comment.
This is the common case: simply wrap span ID in loc.Span()
| """Print a hard-coded message with a prefix, and quote code.""" | ||
| span_id = location.GetSpanId(blame_loc) |
There was a problem hiding this comment.
we can use our function to convert a loc_t to a span_id
Later will remove it from this internal function, but for now it lets us make a small diff
| @@ -1046,7 +1046,7 @@ def _Dispatch(self, node, cmd_st): | |||
| else: | |||
| # Only print warnings, never fatal. | |||
| # Bash oddly only exits 1 for 'return', but no other shell does. | |||
| self.errfmt.PrefixPrint(msg, prefix='warning: ', span_id=tok.span_id) | |||
| self.errfmt.PrefixPrint(msg, 'warning: ', tok) | |||
There was a problem hiding this comment.
Here we do NOT pass loc.Span() -- we just pass the token itself -- because we happen to have it available!
Remember that a Token is a location
Anywhere you would see loc.Span(tok.span_id) -- that's wrong and should just be tok
That's like doing a "double conversion" and is not necessary
The same can be done for Print_() and PrintMessage(), which now take a span_id, but should take a location.