Skip to content
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

[Feature Request] Add code suggestion for PR review comments #14765

Open
delvh opened this issue Feb 21, 2021 · 7 comments
Open

[Feature Request] Add code suggestion for PR review comments #14765

delvh opened this issue Feb 21, 2021 · 7 comments
Labels
proposal/accepted We have reviewed the proposal and agree that it should be implemented like that/at all. topic/pr Issues related to pull requests type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@delvh
Copy link
Member

delvh commented Feb 21, 2021

  • Gitea version (or commit ref): 1.14.0+dev-740-g7118347ba

Description

GitHub supports the option to add code suggestions that can automatically overwrite the current code if committed. The user has the option to add infinitely many suggestions together in a batch before committing all in one single code review commit.
Any code review comment can contain suggestions by simply using

```suggestion
``` (no text here, solely exists so that the quotes at the beginning will be displayed correctly)

The text that gets replaced is the text selected for this comment, resulting in this issue being effectively blocked until #12640 gets merged.
Of course, it would already be possible to implement, but limited to single-line code suggestions for now.

Later on, this feature could even be enhanced to use additional language highlighting, so that the recommendations would even be displayed in the style of the given language. This could look something like this then:

```<language> suggestion

, for example

```go suggestion

This is a useful feature as it allows integrating code suggestions way quicker and without the help of any external tool, where each change has to be manually located first.

Screenshots

Current design on GitHub:
image

behavior when there are multiple suggestions in one comment:
image

@lunny lunny added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Feb 22, 2021
@a1012112796
Copy link
Member

I think this issue is duplicate of #9388.

@delvh
Copy link
Member Author

delvh commented Feb 22, 2021

Well, partially. As @6543 pointed out in the issue description, it could be applied to this issue.
Problem one not covered in that issue: From the issue title, it can't be assumed that code suggestions (as they are called on GitHub) are meant (hence that issue can't be found when someone wants code suggestions to be implemented).
Problem two: That issue currently provides no screenshot of how an existing implementation looks like
Problem three: The issue does not point out that multiple diffs should be able to be combined into a single commit.
If all of that gets moved there, then this issue can be closed as duplicate

@yardenshoham yardenshoham added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Mar 22, 2023
@silverwind silverwind added theme/review proposal/accepted We have reviewed the proposal and agree that it should be implemented like that/at all. labels May 12, 2023
@silverwind

This comment was marked as off-topic.

@silverwind silverwind added topic/pr Issues related to pull requests and removed theme/review labels May 18, 2023
@0dminnimda
Copy link

What is the progress on this one?

@silverwind
Copy link
Member

silverwind commented Nov 2, 2023

```<language> suggestion

I see no need for this. Language can just be determined from the currently edited file for syntax highlighting purpose.

@Glandos
Copy link

Glandos commented Nov 7, 2023

```<language> suggestion

I see no need for this. Language can just be determined from the currently edited file for syntax highlighting purpose.

If the file is an HTML file, and the suggestion is within a <script> tag, then the highlight language is not HTML.

In Vue SFC, it's more complicated, the <script> tag can have a lang attribute, defining script as Javascript, Typescript, or else.

@2bndy5
Copy link

2bndy5 commented Dec 20, 2024

```<language> suggestion

I see no need for this. Language can just be determined from the currently edited file for syntax highlighting purpose.

I would argue that syntax highlighting is not feasible. The resulting code block would be rendered with diff syntax.

- remove this
+ replace with this

Adding syntax highlighting for a different language (rendered within the diff syntax) is an unnecessary complexity and may be counterintuitive to those that are unfamiliar with code suggestion blocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal/accepted We have reviewed the proposal and agree that it should be implemented like that/at all. topic/pr Issues related to pull requests type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

8 participants