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.

Sign up for GitHub

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

Jump to bottom

Create a function to return the git-link itself #69

Closed
wants to merge 1 commit into from
Closed

Create a function to return the git-link itself #69

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 55 additions & 39 deletions git-link.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,8 @@ return (FILENAME . REVISION) otherwise nil."

(defvar magit-buffer-file-name)

(defun git-link--relative-filename ()
(let* ((filename (buffer-file-name (buffer-base-buffer)))
(defun git-link--relative-filename (&optional filename)
(let* ((filename (or filename (buffer-file-name (buffer-base-buffer))))
(dir (git-link--repo-root)))

(when (null filename)
Expand Down Expand Up @@ -666,6 +666,55 @@ return (FILENAME . REVISION) otherwise nil."
(git-link--remote)))

;;;###autoload
(defun git-link-url (file-or-buffer remote &optional start end use-branch-p)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some thoughts/issues here:

  1. I think we should default remote to git-link-default-remote.
  2. If the current buffer is in directory X but this is called like (git-link-url "/path/to/Y/foo.txt" "origin" 10), where Y is a different repository root, it should return a link to Y/foo.txt's repo but now it links to something random in repo X.
  3. What to do with invalid line numbers, e.g., when end is after or equal to start, when start is nil and end is given? Thinking we check for these cases and signal an error.

Copy link
Author

@futuro futuro Mar 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with items 1 and 3, but item 2 strikes me as out of scope for this PR. Specifically, my first thought for handling this is to change every function that interacts with git-link--exec such that they can pass a buffer/directory to run from, thus isolating the side-effect of changing our working directory.

Do you see another way of achieving 2 that only impacts git-link-url?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... but item 2 strikes me as out of scope for this PR.

When calling it as a separate function it cannot depend on state of some buffer. For example, if one creates an emacs package that calls git-link-url the call will depend on the state of some git repo outside the function call so bugs would occur. Make sense?

I agree with items 1 and 3,

Maybe 2/3 is fine? Do they togeather outweigh the bug?

Do you see another way of achieving 2 that only impacts git-link-url?

I think this is part of the reason why this function has taken so long to exist. If I remember correctly you have to change a bit more than meets the eye. It has been since Dec or so since I had a look at the code. Will have to wait to look again. But if you think you gotz a solution then add it. But if you are concerned about your time then maybe wait 'till I can have an in-depth look again.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also test coverage not that good. Exhibit A.

"Create a URL for the FILE-OR-BUFFER's location at REMOTE.

When given START and END, include the current line number or
active region.

When given USE-BRANCH-P, force the use of the branch name in the
resultant url."
(let* ((filename (git-link--relative-filename
(if (bufferp file-or-buffer)
(buffer-file-name file-or-buffer)
file-or-buffer)))
(remote (or remote git-link-default-remote))
(remote-url (git-link--remote-url remote))
(remote-info (git-link--parse-remote remote-url))
(branch (git-link--branch))
(commit (git-link--commit))
(handler (git-link--handler git-link-remote-alist (car remote-info))))
(cond ((null remote-url)
(user-error "Remote `%s' not found" remote))
((null filename)
(user-error "Can't figure out what to link to"))
((null (car remote-info))
(user-error "Remote `%s' contains an unsupported URL" remote))
((not (functionp handler))
(user-error "No handler found for %s" (car remote-info)))
;; TODO: null ret val
(t
(let* ((vc-revision (git-link--parse-vc-revision filename))
(hostname (car remote-info))
(dirname (cadr remote-info))
(branch (if (and (not use-branch-p)
(or (git-link--using-git-timemachine)
(git-link--using-magit-blob-mode)
vc-revision
git-link-use-commit))
nil
(url-hexify-string branch)))
(filename (if vc-revision (car vc-revision) filename))
(commit (if vc-revision (car vc-revision) commit)))
(funcall handler
hostname
dirname
filename
branch
commit
start
end))))))

(defun git-link (remote start end)
"Create a URL representing the current buffer's location in its
GitHub/Bitbucket/GitLab/... repository at the current line number
Expand All @@ -688,43 +737,10 @@ Defaults to \"origin\"."
(list remote nil nil)
(list remote (car region) (cadr region))))))

(let (filename branch commit handler remote-info (remote-url (git-link--remote-url remote)))
(if (null remote-url)
(message "Remote `%s' not found" remote)

(setq remote-info (git-link--parse-remote remote-url)
filename (git-link--relative-filename)
branch (git-link--branch)
commit (git-link--commit)
handler (git-link--handler git-link-remote-alist (car remote-info)))

(cond ((null filename)
(message "Can't figure out what to link to"))
((null (car remote-info))
(message "Remote `%s' contains an unsupported URL" remote))
((not (functionp handler))
(message "No handler found for %s" (car remote-info)))
;; TODO: null ret val
(t
(let ((vc-revison (git-link--parse-vc-revision filename)))
(when vc-revison
(setq filename (car vc-revison)
commit (cdr vc-revison)))

(git-link--new
(funcall handler
(car remote-info)
(cadr remote-info)
filename
(if (or (git-link--using-git-timemachine)
(git-link--using-magit-blob-mode)
vc-revison
git-link-use-commit)
nil
(url-hexify-string branch))
commit
start
end))))))))
(condition-case err
(git-link--new (git-link-url buffer-file-name remote start end))
(user-error
(message (cadr err)))))

;;;###autoload
(defun git-link-commit (remote)
Expand Down