View Issue Details

IDProjectCategoryView StatusLast Update
0000898LDMud 3.6Generalpublic2022-01-09 20:35
Reporterparadox Assigned ToGnomi  
PrioritynormalSeverityminorReproducibilityN/A
Status resolvedResolutionfixed 
PlatformN/AOSN/AOS VersionN/A
Product Version3.6.4 
Target Version3.6.5Fixed in Version3.6.5 
Summary0000898: Provide line number as an argument to master's log_error lfun hook.
DescriptionPresently, 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 ReproduceCompile an LPC file with warnings or errors, observe that `log_error` in master.c is called, but without providing a line number argument.
TagsNo tags attached.

Activities

paradox

2021-10-17 20:08

reporter   ~0002660

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!

paradox

2021-10-22 21:35

reporter   ~0002662

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.

paradox

2021-10-22 22:16

reporter   ~0002663

> 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!

paradox

2022-01-08 21:50

reporter   ~0002673

This was resolved by Gnomi merging https://github.com/ldmud/ldmud/commit/7f14d6028d7b8ab5f5f1955b739f2464660dbca8 to master.

Issue History

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