astahlman / magit-gh-comments Goto Github PK
View Code? Open in Web Editor NEWComment on Github pull requests via magit
magit-gh-comments's People
Contributors
Watchers
magit-gh-comments's Issues
Ability to delete comments
As a user, I want to place my point on a comment in a magit-diff buffer and delete my comment.
Comments that are "outdated" should be hidden, by default
Comments on an old revision or that have been resolved should be collapsed unless specifically expanded.
Point is not preserved when refreshing diff buffer
Steps to reproduce:
Put point on a random line in a hunk. Run magit-refresh interactively. Point is now at the bottom of the buffer
Resolved conversations should be marked as such
They look like normal comments, currently. We should keep them collapsed by default, and mark them as "Resolved by "
Off-by-one when posting comments to Github
When fetching and display comments in a Magit diff buff, the comments appear one below their expected position. I'm not yet sure whether they are being posted to Github with the wrong line number or if the comment display logic has a bug.
Pending reviews should be marked as such
The review buffer currently shows pending, top-level, review comments, but it doesn't include any of the pending comments attached to the review. It also doesn't indicate that the review as "pending." It should be obvious that a review is pending if it hasn't been submitted, and we should also show all pending comments .
Comments should be displayed in chronological order
Multiple comments on the same line/thread should be order by ascending creation time (i.e., oldest at the top, newest at the bottom)
submit-review should fail if there is no body or comment
See this comment - I created it with submit-review without any comments or body.
Add a [Full diff] jumper at the bottom of the diffstat section
It's not obvious how to view the diff without some sort of button that explicitly says "View diff".
PR view should show list of commits
Should we make this a timeline like view, like Github does? Or put them in their own section?
I'm leaning towards the latter, to start with.
Ability to cycle through comments
As a user, I would like to jump forward or backward through the comments in a diff buffer with a single keystroke.
Ability to edit comments
As a user, I want to place point on a comment in a magit-diff buffer and edit the body of the comment.
Add placeholder text if the PR has no description
It should say No description provided. in italics, like on the Github UI.
Minibuf should show hints when point is on an editable section
Half-baked idea: when point is on something that can be interacted with (e.g., a comment), show the keybindings and/or actions available. This should be doable by setting the kbd-help text property, i.e.,
(propertize "Some comment" 'kbd-help "[C-c e] Edit \n[C-c d] Delete")
All comments are misplaced when opening PR diff
When the user re-opens a PR diff from Magit, all of the comments appear at very top of the buffer until the user interactively runs `magit-gh-refresh-comments'. The comments should appear in their correct position when the buffer is opened.
Can't jump to files that have been renamed
See the PR for #27, for which the diffstat looks like this:
Files changed
4 files changed, 51 insertions(+), 37 deletions(-)
magit-gh-comments-github.el | 39 ++++++++++++-------------
magit-gh-utils.el => magit-gh-comments-utils.el | 9 +++---
magit-gh-comments.el | 36 ++++++++++++++++-------
test/magit-gh-comments-test.el | 4 +--
Following the jumper magit-gh-utils.el => magit-gh-comments-utils.el produces this backtrace:
Debugger entered--Lisp error: (error "Couldn’t find a file in the diff named ‘magit-gh-utils.el => magit-gh-comments-utils.el’")
signal(error ("Couldn’t find a file in the diff named ‘magit-gh-utils.el => magit-gh-comments-utils.el’"))
error("Couldn't find a file in the diff named `%s'" "magit-gh-utils.el => magit-gh-comments-utils.el")
(if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath))
(let* ((section (and t (magit-get-section (cons (cons (quote file) filepath) (quote (...))))))) (if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath)))
(closure ((pr . [cl-struct-magit-gh-pr "astahlman" "magit-gh-comments" 27 "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea" "Refactoring: represent review as struct instead of alist" "open" "Return a list of structs from `magit-gh-list-reviews' instead of a\nlist of alists. We're now consistent in our internal representation of\nreviews as using the `magit-gh-review' struct.\n\nI also switched to using hashtables in the implementation of\n`magit-gh-list-reviews'. Dealing with three levels of nesting in the\ngrouped list of alists was getting hard to reason about."]) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (keymap keymap (return closure #1 nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t ...))) (if section (progn (goto-char ...) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath))))) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (end . 638) (start . 591) (match) (_args ("diff" "--stat" "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea")) t) nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t (magit-get-section (cons (cons ... filepath) (quote ...)))))) (if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath))))()
funcall-interactively((closure ((pr . [cl-struct-magit-gh-pr "astahlman" "magit-gh-comments" 27 "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea" "Refactoring: represent review as struct instead of alist" "open" "Return a list of structs from `magit-gh-list-reviews' instead of a\nlist of alists. We're now consistent in our internal representation of\nreviews as using the `magit-gh-review' struct.\n\nI also switched to using hashtables in the implementation of\n`magit-gh-list-reviews'. Dealing with three levels of nesting in the\ngrouped list of alists was getting hard to reason about."]) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (keymap keymap (return closure #1 nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t ...))) (if section (progn (goto-char ...) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath))))) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (end . 638) (start . 591) (match) (_args ("diff" "--stat" "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea")) t) nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t (magit-get-section (cons (cons ... filepath) (quote ...)))))) (if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath)))))
#<subr call-interactively>((closure ((pr . [cl-struct-magit-gh-pr "astahlman" "magit-gh-comments" 27 "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea" "Refactoring: represent review as struct instead of alist" "open" "Return a list of structs from `magit-gh-list-reviews' instead of a\nlist of alists. We're now consistent in our internal representation of\nreviews as using the `magit-gh-review' struct.\n\nI also switched to using hashtables in the implementation of\n`magit-gh-list-reviews'. Dealing with three levels of nesting in the\ngrouped list of alists was getting hard to reason about."]) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (keymap keymap (return closure #1 nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t ...))) (if section (progn (goto-char ...) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath))))) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (end . 638) (start . 591) (match) (_args ("diff" "--stat" "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea")) t) nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t (magit-get-section (cons (cons ... filepath) (quote ...)))))) (if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath)))) nil nil)
apply(#<subr call-interactively> (closure ((pr . [cl-struct-magit-gh-pr "astahlman" "magit-gh-comments" 27 "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea" "Refactoring: represent review as struct instead of alist" "open" "Return a list of structs from `magit-gh-list-reviews' instead of a\nlist of alists. We're now consistent in our internal representation of\nreviews as using the `magit-gh-review' struct.\n\nI also switched to using hashtables in the implementation of\n`magit-gh-list-reviews'. Dealing with three levels of nesting in the\ngrouped list of alists was getting hard to reason about."]) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (keymap keymap (return closure #1 nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t ...))) (if section (progn (goto-char ...) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath))))) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (end . 638) (start . 591) (match) (_args ("diff" "--stat" "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea")) t) nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t (magit-get-section (cons (cons ... filepath) (quote ...)))))) (if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath)))) (nil nil))
(let ((ido-cr+-current-command command)) (apply orig-fun command args))
call-interactively@ido-cr+-record-current-command(#<subr call-interactively> (closure ((pr . [cl-struct-magit-gh-pr "astahlman" "magit-gh-comments" 27 "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea" "Refactoring: represent review as struct instead of alist" "open" "Return a list of structs from `magit-gh-list-reviews' instead of a\nlist of alists. We're now consistent in our internal representation of\nreviews as using the `magit-gh-review' struct.\n\nI also switched to using hashtables in the implementation of\n`magit-gh-list-reviews'. Dealing with three levels of nesting in the\ngrouped list of alists was getting hard to reason about."]) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (keymap keymap (return closure #1 nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t ...))) (if section (progn (goto-char ...) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath))))) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (end . 638) (start . 591) (match) (_args ("diff" "--stat" "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea")) t) nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t (magit-get-section (cons (cons ... filepath) (quote ...)))))) (if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath)))) nil nil)
apply(call-interactively@ido-cr+-record-current-command #<subr call-interactively> ((closure ((pr . [cl-struct-magit-gh-pr "astahlman" "magit-gh-comments" 27 "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea" "Refactoring: represent review as struct instead of alist" "open" "Return a list of structs from `magit-gh-list-reviews' instead of a\nlist of alists. We're now consistent in our internal representation of\nreviews as using the `magit-gh-review' struct.\n\nI also switched to using hashtables in the implementation of\n`magit-gh-list-reviews'. Dealing with three levels of nesting in the\ngrouped list of alists was getting hard to reason about."]) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (keymap keymap (return closure #2 nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section ...)) (if section (progn ... ...) (error "Couldn't find a file in the diff named `%s'" filepath))))) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (end . 638) (start . 591) (match) (_args ("diff" "--stat" "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea")) t) nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t (magit-get-section (cons ... ...))))) (if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath)))) nil nil))
call-interactively((closure ((pr . [cl-struct-magit-gh-pr "astahlman" "magit-gh-comments" 27 "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea" "Refactoring: represent review as struct instead of alist" "open" "Return a list of structs from `magit-gh-list-reviews' instead of a\nlist of alists. We're now consistent in our internal representation of\nreviews as using the `magit-gh-review' struct.\n\nI also switched to using hashtables in the implementation of\n`magit-gh-list-reviews'. Dealing with three levels of nesting in the\ngrouped list of alists was getting hard to reason about."]) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (keymap keymap (return closure #1 nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t ...))) (if section (progn (goto-char ...) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath))))) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (end . 638) (start . 591) (match) (_args ("diff" "--stat" "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea")) t) nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t (magit-get-section (cons (cons ... filepath) (quote ...)))))) (if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath)))) nil nil)
command-execute((closure ((pr . [cl-struct-magit-gh-pr "astahlman" "magit-gh-comments" 27 "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea" "Refactoring: represent review as struct instead of alist" "open" "Return a list of structs from `magit-gh-list-reviews' instead of a\nlist of alists. We're now consistent in our internal representation of\nreviews as using the `magit-gh-review' struct.\n\nI also switched to using hashtables in the implementation of\n`magit-gh-list-reviews'. Dealing with three levels of nesting in the\ngrouped list of alists was getting hard to reason about."]) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (keymap keymap (return closure #1 nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t ...))) (if section (progn (goto-char ...) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath))))) (diff-range . "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea") (filepath . "magit-gh-utils.el => magit-gh-comments-utils.el") (end . 638) (start . 591) (match) (_args ("diff" "--stat" "cc7e2ce78c5b27b4545848d2708f83bbb3651d4a..a046d6002db1ea1f14af20c956a559f1837daaea")) t) nil (interactive) (magit-diff diff-range) (set (make-local-variable (quote magit-gh--current-pr)) pr) (let* ((section (and t (magit-get-section (cons (cons ... filepath) (quote ...)))))) (if section (progn (goto-char (eieio-oref section (quote start))) (recenter 0)) (error "Couldn't find a file in the diff named `%s'" filepath)))))
Ability to reply to comments
As a user, I would like to reply to a comment by placing point on the comment and pressing a key.
Ability to edit PR description
I'd like to be able to edit the description in a PR buffer and push the changes to Github.
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.