Add inline commit coments

[ghost]

Adds inline comments to commits. Inline commit comments can be considered new feature and does not use the existing
functionality of comments, issues or attachments.

/claim #4898

[wxiaoguang]

A quick look, still many problems:

• Some functions have security problems (for example: DeleteCommitAttachment can delete any attachment by UUID)
• Some designs are strange (for example: there is no ctx.Session.Get("attachmentsMaps") in existing code base, it looks not right)
• Too many if-else logic (in go and tmpl code) for "different pages", it is quite fragile
• Incorrect tmpl variables (for example: ctx: ctx.Data can't be right)
• Incorrect logic (for example: CancelCommitComment)
• Many functions have complex logic, so they need tests.

So overall it doesn't look good enough. If it can continue, please review the changed code line by line and add necessary tests. (by the way: I only take a quick look for the code, I haven't really thought about whether the design is feasible).

[lunny]

What's the design how to cooperate with attachment table? The attachment maybe GC because all foreign indexes are zero?

[lunny]

And there is an edge case, if a commit was GC, the commit comments and related attachments, reactions need to be deleted as well.

[wxiaoguang]

The following items are resolved

The fixes are not right. You shouldn't use a global variable like attachmentsMap = make(AttachmentMap). It is wrong.

I don't know whether the code is written by you or by AI, but overall, it doesn't look right. Too many problems in code, and it's impossible to review.

I would suggest you to #4898 (comment) :

This feature is not an easy task. Copying-pasting or AI-coding won't work. If a PR contains too many problems, it would be very difficult to review and unlikely to be merged.

For new contributors, if you'd like to try, I would suggest to start from some "good first issues", you can try to do something in the modules that are related to this feature, and get familiar to the code base, then start the challenging work.

[wxiaoguang]

How could a global variable be right for the persistent data? How to handle instance restart? How to handle more than one instances in a cluster?

[wxiaoguang]

No, it is definitely wrong.

1. If the user opens 2 windows (tabs), they share the same attachmentsMap, then the attachments become a mess.
2. If the user is writing a comment, and the server restarts, then the uploaded "temp" attachments are lost.
3. If the Gitea server are running in a cluster, then you have many different attachmentsMap variables, then the attachments become a mess.
4. Your design breaks Gitea's attachment management. See checkStorage(&checkStorageOptions{Attachments: true}), then all your attachments will be removed since they are not in the attachment table.

I would reiterate my suggestion again: it is not an easy task, please study Gitea's code carefully and start from small PRs (some good first issues) before proposing such a large PR if you are new to the code base.

[wxiaoguang]

If the user is writing a comment, and the server restarts, then the uploaded "temp" attachments are lost.

In the existing gitea instance, the uploaded "temp" attachments are also lost upon server restart or page refresh.

Are you sure? I never mentioned "page refresh", that's another problem which relates to "editor draft".

For server restarts, it doesn't affect the users who are writing and uploading:

I need clarification on the requirements for inline comments. The current gitea instance does not seem to be using persistent session store or storing session data in database, otherwise refreshing the comments for pullrequests would reload the attachment data from session store.

Please understand the code first. I think I have suggested many times: you can start from some easy tasks to get familiar with the code base.

[wxiaoguang]

If the user is writing a comment, and the server restarts, then the uploaded "temp" attachments are lost.

In the existing gitea instance, the uploaded "temp" attachments are also lost upon server restart or page refresh.

Are you sure? I never mentioned "page refresh", that's another problem which relates to "editor draft".

If you are interested in the "editor draft" topic, you can read my another issue: [Summary] Improve issue/PR comment editing experience #23290

[wxiaoguang]

Ideally session data can be stored in persistence storage i.e database table to prevent session data getting lost on restart.

Gitea's session data is stored in persistence storage (file) by default installation. And there are many providers, only the "memory" provider doesn't persist the session data.

gitea/routers/install/install.go

Line 449 in 1740d36

cfg.Section("session").Key("PROVIDER").SetValue("file")

gitea/custom/conf/app.example.ini

Lines 1909 to 1911 in 1740d36

	;; Either "memory", "file", "redis", "db", "mysql", "couchbase", "memcache" or "postgres"
	;; Default is "memory". "db" will reuse the configuration in [database]
	;PROVIDER = memory

[wxiaoguang]

I have spent enough time on reviewing this PR and arguing about the details, and it seems that you have your own strong opinions. I have suggested many times: start from some easy tasks and get familiar with the code base first.

And sorry I don't understand what your "approach" would be. So I can't comment more.

[wxiaoguang]

Regarding the existing approach. I would add commit_attachments table aligning with the attachment_table

Questions:

1. checkStorage(&checkStorageOptions{Attachments: true}) should keep all the attachments, how to.
2. The URL in the comment is /attachments/xxx which means http://host/owner/repo/attachments/xxx, and the markdown editor always generates /attachments/xxx or attachments/xxx links (hard-coded), how to deal with it.
3. What are the Pros and Cons of "adding a new attachment table". What if we need to store attachments for more "comment-like" features, e.g.: GitHub-like discussion. Keep adding more tables, or use one consistent table.

[wxiaoguang]

Thank you for the update but sorry it looks wrong. At the moment I don't have time and can't help more.