Comments on pull files#2583
Conversation
lunny
added
type/feature
lunny
force-pushed
the
lunny/commit_line_comment
branch
from
5e9279a to
21a2b4f
Compare
| <noscript><link rel="stylesheet" href="{{AppSubUrl}}/vendor/assets/font-awesome/css/font-awesome.min.css"></noscript> | ||
| <link rel="stylesheet" href="{{AppSubUrl}}/vendor/assets/octicons/octicons.min.css"> | ||
|
|
||
| <link rel="stylesheet" href="{{AppSubUrl}}/css/main.css"> |
There was a problem hiding this comment.
Why new css file? I think these css additions need to be done in less file and built into minimized index.css
| CommitID int64 | ||
| Line int64 | ||
| TreePath string | ||
| Line int64 // + is left; - is right |
There was a problem hiding this comment.
Instead of using +/- we could go with gitea's style and add a type attribute with possible valuesaddition, deletion.
There was a problem hiding this comment.
Don't want to add an extra column, so I define the line, if it's greater than zero, it should the previous code line before the pull's commits. If it's less than zero, it should the line on this pull's commits.
There was a problem hiding this comment.
If everyone's happy with this I won't obstruct.
There was a problem hiding this comment.
IMO, if you use +/-, it should be - for previous line and + for new line.
| } | ||
|
|
||
| /* global showdown */ | ||
| function createCodeComment() { |
There was a problem hiding this comment.
Please replace javascript comment box creation by using an existing hidden comment template placed in html.
Github does it that way and IMO it's far better.
| <script src="{{AppSubUrl}}/vendor/plugins/clipboard/clipboard.min.js"></script> | ||
| <script src="{{AppSubUrl}}/vendor/plugins/vue/vue.min.js"></script> | ||
|
|
||
| <script src="{{AppSubUrl}}/vendor/plugins/showdown/showdown.min.js"></script> |
There was a problem hiding this comment.
Shouldn't we already have a markdown converter, since we are doing the exact same thing on issues/comments?
| /* global showdown */ | ||
| function createCodeComment() { | ||
| const texts = { | ||
| btn: "Comment Code", |
There was a problem hiding this comment.
Shouldn't we use locales for these?
|
@lunny Got any screenshots? And are there visible when viewing a specific commit? (like here: https://try.gitea.io/gitea/gitea/commit/b7da5a6cb7c725232c1053a1ca705a6ac0dad467 ) |
tboerger
added
the
lgtm/need 2
|
@bkcsoft currently only for pull files. For specific commit should be another PR after this merged. |
|
What's the status on this? |
|
Nevermind, it looks like we're doing this on #3748 instead. Shouldn't this be closed? |
|
@thehowl yes. |
8 participants
First step on comment in pull files, and will resolved #124, but still WIP.