potassco / potassco/constraint-handler
Improvement of warnings.
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 3
- Forks
- 0
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 20
Description
here are some thoughts I had while looking at the warnings again.
If possible, I think the warnings should be streamlined into a common structure. Right now, warning/1 takes a tuple and can basically display anything, but I feel a little bit more structure could help.
Additionally, I think the statement identifiers are a good concept, but they aren't used consistently everywhere. For instance, for variable_declare I have to provide a statement identifier, but for variable_domain I don't. Instead, during the actual assignment the identifier of the declare is used. but the same doesn't hold true for set assignments where the user has to provide a statement identifier as well.
Given that a user technically can define a domain over multiple statements, the user may want to know which of these fails during an assignment. If instead the user shouldn't be allowed to spread a domain over multiple statements, then an identifier could be used to provide a meaningful message.
Likewise (as seen in variables.lp):
warning((X,"was defined twice:",E1,E2)) :- variable_define(CNAME1,X,E1), variable_define(CNAME2,X,E2),
(CNAME1,E1) < (CNAME2,E2).
Here, I think E1 and E2 provide less value to the user than CNAME1 and CNAME2 given the user provides meaningful statement identifiers. E1 and E2 might be very complex expressions that are difficult to read while CNAME1 and CNAME2 can be used to easily verify duplication and identify the wrong statement.
The same goes for variable_declare where it can be even worse. Consider this:
variable_declare(some_id, y, fromFacts). % In one file
variable_declare(some_id_2, y, fromFacts). % In some other file
The user would receive the warning warning((y,"was declared twice:",fromFacts,fromFacts)) which doesn't help much. If the user instead received warning((y,"was declared twice:", some_id, some_id_2)) they could find the specific declarations immediately.
Furthermore, I feel that right now a user would have to know very specific error messages in order to identify or filter for specific errors. While this is probably not as common here, it may still help to instead provide names of error types. Otherwise, re-wording of messages may break a user's filter even though the error type and content otherwise remains the same.
Generally, if possible, I would suggest to always propagate the statement names, use statement names everywhere and then do something like:
warning(Type, Identifier, Details).
| Name | Description |
|---|---|
Type |
The type of error/warning that was issued. |
Identifier |
The identifier of the statement that caused the warning. |
Details |
Here we could put the details like before. |
We could then have types such as duplicate_variable_definition_error,duplicate_variable_declaration_error or expression_evaluation_error (or use whatever naming-convention you like). A user can then use these to filter for certain error types without relying on the specific wording of the error messages.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing warning/1 and statement-identifier propagation across the warning implementations, including variable.lp and the variable_declare, variable_domain, and set-assignment cases mentioned here. Define the desired structured warning fields and identifier behavior, then verify that duplicate declarations and definitions expose the useful statement identifiers and warning types.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100