proselint icon indicating copy to clipboard operation
proselint copied to clipboard

Dialogue attribution and adverbs re: #106

Open Serene-Arc opened this issue 4 years ago • 8 comments

I implemented the rule from Steven King about adverbs in dialogue atribution, as described in issue #106

It's a fairly simple regex string that finds a piece of dialogue and checks to see if there is an adverb in the following phrase. I think it's fairly fragile but it's worked on the example texts that I've tested it with.

This is my first pull request and my first time contributing to a project that isn't my own so any help would be appreciated :)

Serene-Arc avatar Jun 08 '20 00:06 Serene-Arc

@Serene-Arc I know this was a while ago, but are you still willing to work on it? Let me know. Thanks for your submission either way.

Nytelife26 avatar May 29 '21 14:05 Nytelife26

@Nytelife26 Hi! I am still willing to work on it :) This is a great tool

Serene-Arc avatar May 29 '21 23:05 Serene-Arc

I am still willing to work on it :)

Excellent! Thank you very much for your time and efforts. In that case, a few things need to be done:

  1. Can you rebase this PR over main, please?
  2. Unit tests for this rule will also need to be written to ensure there are as few false positives as possible

Nytelife26 avatar May 30 '21 12:05 Nytelife26

Sure, I'll get to it as soon as I can

Serene-Arc avatar May 30 '21 22:05 Serene-Arc

Codecov Report

Merging #1080 (bc183cd) into main (ae2161b) will increase coverage by 3.53%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1080      +/-   ##
==========================================
+ Coverage   90.19%   93.72%   +3.53%     
==========================================
  Files          83       84       +1     
  Lines        1203     1275      +72     
==========================================
+ Hits         1085     1195     +110     
+ Misses        118       80      -38     
Flag Coverage Δ
macos-latest 93.72% <100.00%> (+3.53%) :arrow_up:
py3.6 93.13% <100.00%> (+3.91%) :arrow_up:
py3.7 93.13% <100.00%> (+3.91%) :arrow_up:
py3.8 93.72% <100.00%> (+3.53%) :arrow_up:
py3.9 93.72% <100.00%> (+3.53%) :arrow_up:
pypypy3 93.13% <100.00%> (+3.91%) :arrow_up:
ubuntu-latest 93.72% <100.00%> (+3.53%) :arrow_up:
windows-latest 93.72% <100.00%> (+3.53%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
proselint/checks/redundancy/adverb_dialogue.py 100.00% <100.00%> (ø)
proselint/checks/lexical_illusions/misc.py 100.00% <0.00%> (ø)
proselint/tools.py 84.92% <0.00%> (+7.35%) :arrow_up:
proselint/command_line.py 71.15% <0.00%> (+26.92%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ae2161b...bc183cd. Read the comment docs.

codecov[bot] avatar May 31 '21 01:05 codecov[bot]

I went ahead and applied those suggested changes for you, @Serene-Arc. Any chance you could create tests for this new check at some point, and perhaps rewrite the file docstrings to be more descriptive like in other checks?

Nytelife26 avatar Jul 04 '21 15:07 Nytelife26

@Nytelife26 Ah, thanks! I'd already applied the suggested changes but I'll overwrite them with these. I've got some tests written, just been busy with some other stuff sorry, so I'll get them done soon and bring in the docstrings to be in line with the rest of the project.

Serene-Arc avatar Jul 04 '21 23:07 Serene-Arc

@Nytelife26 Ah, thanks! I'd already applied the suggested changes but I'll overwrite them with these. I've got some tests written, just been busy with some other stuff sorry, so I'll get them done soon and bring in the docstrings to be in line with the rest of the project.

That's absolutely fine. Continue at your own pace - I just had some free time and thought I'd give you a helping hand :)

Let me know if you need anything, and good luck with your other projects (or whatever it is you've been busy with).

Nytelife26 avatar Jul 05 '21 12:07 Nytelife26