search

pwntester / octo.nvim

Edit and review GitHub issues and pull requests from the comfort of your favorite editor
MIT License
2.43k stars 137 forks source link

Suggestions ignore comments outside of the suggestion #159

Closed TaylorBeeston closed 3 years ago

TaylorBeeston commented 3 years ago

First off: this plugin is incredible. Thank you so much for making this!

I make pretty extensive use of suggestions in my PR reviews, and very often those suggestions come with comments:

\```suggestion
-- Change this code
\```
This change is important because...

Right now, when adding that bottom comment with octo, it will kind of look like it's being added, but get lopped off when actually submitting the review. I can actually see this before submitting by popping open the browser and looking at my pending suggestion: it will have the comment text removed.

For now, I can happily just reply to my own suggestions, or just use the browser editor to add those back in, but it would really help me out with my PR workflow for those guys to just work =)

Thank you again for this incredible plugin! Feels like now that 0.5 is stable I'm finding all these wonderful gems that I could have been using for months 🙃

pwntester commented 3 years ago

Hi, thanks for the report, glad you like octo!

I tried reproducing the issue, but my comment does not disappear (eg: https://github.com/pwntester/octo.nvim/pull/14#discussion_r666170073)

Im wondering if there is any character in the comment that is not correctly escaped and may cause the comment to break 🤔 Can you please post the complete body of the comment (suggestion + additional text) to see If I can reproduce it

TaylorBeeston commented 3 years ago

Hmm... This worked out when I reviewed the same PR on this repo here 🙃

The codebase I have been having this issue with is private, so perhaps it is related to that? I think this is probably safe to close though, as it's clearly not as big of a problem as I had thought, and would likely be very difficult to track down and confirm a fix since it's difficult to reproduce

pwntester commented 3 years ago

Sure, will be happy to reopen if you find this issue in an open repo that we can reproduce :)