-
Notifications
You must be signed in to change notification settings - Fork 249
Flag plural "damages" when not used for judicial compensation #2365
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
base: master
Are you sure you want to change the base?
Conversation
elijah-potter
left a comment
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.
A few nits, but overall this is very close. Thanks!
harper-core/src/linting/mod.rs
Outdated
| /// A string with ANSI escape codes where: | ||
| /// - Context tokens are dimmed before and after the matched tokens in normal weight. | ||
| /// - Markup and formatting text hidden in whitespace tokens is filtered out. | ||
| pub fn format_lint_match( |
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.
I'm hesitant to include this right now, since it doesn't seem to be in use anywhere.
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.
I'm hesitant to include this right now, since it doesn't seem to be in use anywhere.
Oh sorry that has its own PR. I use it in every new linter I work on but remove it before making the PR. Which I see I forgot to do this time sorry. Officially in today's inbox now.
harper-core/src/linting/damages.rs
Outdated
| } | ||
| } | ||
|
|
||
| // TODO: Is this functional-style code better than the for loop version above? |
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.
I personally find the functional code easier to read. I imagine they compile down to something similar once LLVM gets its hands on it. If you prefer the loop version, would you mind removing this comment for clarity?
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.
I personally find the functional code easier to read. I imagine they compile down to something similar once LLVM gets its hands on it. If you prefer the loop version, would you mind removing this comment for clarity?
I often find functional harder to reason about but when I get such problems working with loops I love to see how our robotic overlord friends can refactor them into functional code, which I find cool and interesting.
I let both in specifically to get your feedback in PR review so I'll keep the functional version - thanks!
Issues
Resolves #954
Description
(Work in progress)
Tries to determine if uses of "damages" are noun senses that don't refer to compensation awarded by a court etc.
How Has This Been Tested?
Unit tests based on real-world sentences from GitHub and maybe elsewhere on the internet.
Checklist