View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0000898 | LDMud 3.6 | General | public | 2021-10-17 19:51 | 2022-01-09 20:35 |
Reporter | paradox | Assigned To | Gnomi | ||
Priority | normal | Severity | minor | Reproducibility | N/A |
Status | resolved | Resolution | fixed | ||
Platform | N/A | OS | N/A | OS Version | N/A |
Product Version | 3.6.4 | ||||
Target Version | 3.6.5 | Fixed in Version | 3.6.5 | ||
Summary | 0000898: Provide line number as an argument to master's log_error lfun hook. | ||||
Description | Presently, the `log_error` lfun called in master for compiler errors and warnings uses the function signature: ``` void log_error(string file, string err, int warn) ``` The "err" argument most typically[0] is a string that includes the file at fault, the error line number, and some context: ``` sprintf(buff, "%s line %d%s: %s\n", error_file, line, context, what); ``` I would like to be able to access the line number _without_ having to parse the "err" string. This closer matches the `runtime_error` and `runtime_warning` hooks that receive the line number as one of the arguments to the master `lfun` called by the driver. [0]: https://github.com/ldmud/ldmud/blob/cf009941c0aefa593dc353fe62d938a3b0b4bcb5/src/simulate.c#L1399 | ||||
Steps To Reproduce | Compile an LPC file with warnings or errors, observe that `log_error` in master.c is called, but without providing a line number argument. | ||||
Tags | No tags attached. | ||||
|
If you're interested in helping a driver neophyte implement this themselves I'd be willing to try and to submit a PR. If you have any notes on "gotchas" I should watch out for (thinking in particular, for legacy instances not expecting this argument) that would be helpful! |
|
I've put up a "draft" PR here: https://github.com/ldmud/ldmud/pull/63 I think the main missing piece is test coverage. I will look at that next, advice welcome! I did some quick manual testing to ensure that this patch still worked on a driver that had a master ob that didn't expect the new argument to its `log_error` function and saw no problems. I also tested that a master ob that _did_ expect the new argument got the right value and it seemed to be working correctly. |
|
> I think the main missing piece is test coverage. I will look at that next, advice welcome! I made a basic unit test (https://github.com/ldmud/ldmud/pull/63/commits/fcdf4fca5238a1fc2f26fecc00f2d106d83f6e41) that appears to pass locally when I run it. If there's some other test configurations to consider I'm happy to try! |
|
This was resolved by Gnomi merging https://github.com/ldmud/ldmud/commit/7f14d6028d7b8ab5f5f1955b739f2464660dbca8 to master. |
Date Modified | Username | Field | Change |
---|---|---|---|
2021-10-17 19:51 | paradox | New Issue | |
2021-10-17 20:08 | paradox | Note Added: 0002660 | |
2021-10-22 21:35 | paradox | Note Added: 0002662 | |
2021-10-22 22:16 | paradox | Note Added: 0002663 | |
2022-01-08 21:50 | paradox | Note Added: 0002673 | |
2022-01-08 22:02 | zesstra | Project | LDMud 3.5 => LDMud 3.6 |
2022-01-08 22:02 | zesstra | Category | Other => General |
2022-01-08 22:03 | zesstra | Assigned To | => Gnomi |
2022-01-08 22:03 | zesstra | Status | new => closed |
2022-01-08 22:03 | zesstra | Resolution | open => fixed |
2022-01-08 22:03 | zesstra | Product Version | => 3.6.4 |
2022-01-08 22:03 | zesstra | Fixed in Version | => 3.6.5 |
2022-01-08 22:03 | zesstra | Target Version | => 3.6.5 |
2022-01-09 20:35 | Gnomi | Status | closed => resolved |