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

Use transient.el #10

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Use transient.el #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8c2d175
Add Makefile
tarsius Feb 6, 2025
831bc3b
Require magit-popup
tarsius Feb 6, 2025
9ec3f51
Require p4 at runtime
tarsius Feb 6, 2025
f9fa16f
magit-p4/insert-job: Fix quoting in docstring
tarsius Feb 6, 2025
d244f39
Fix declaration for find-lisp-find-files-internal
tarsius Feb 6, 2025
6abb8ae
Explicitly set lexical-binding
tarsius Feb 6, 2025
85275dc
Enable lexical-binding and fix a bug thus revealed
tarsius Feb 6, 2025
91081a3
Stop using obsolete magit-p4-process-kill-on-abort
tarsius Feb 6, 2025
cc4f71c
Initial update to use transient instead of magit-popup
Jan 26, 2025
eef285a
Fully port to transient and recent magit versions
Jan 26, 2025
4f5cb85
Update README to reflect recent changes
Jan 29, 2025
9742202
Pull request feedback
Jan 30, 2025
3c8c34e
[fixup] use eval-and-compile
tarsius Feb 6, 2025
7eaff36
Remove unnecessary dependencies on find-lisp and bind-key
mathrick Feb 18, 2025
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
65 changes: 65 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
-include .config.mk

PKG = magit-p4

ELS = $(PKG).el
ELCS = $(ELS:.el=.elc)

DEPS = compat
DEPS += llama
DEPS += magit/lisp
DEPS += p4
DEPS += seq
DEPS += transient/lisp
DEPS += with-editor/lisp

EMACS ?= emacs
EMACS_ARGS ?=

LOAD_PATH ?= $(addprefix -L ../,$(DEPS))
LOAD_PATH += -L .

all: lisp

help:
$(info make all - generate byte-code and autoloads)
$(info make lisp - generate byte-code and autoloads)
$(info make redo - re-generate byte-code and autoloads)
$(info make clean - remove generated files)
@printf "\n"

redo: clean lisp

lisp: $(ELCS) loaddefs check-declare

loaddefs: $(PKG)-autoloads.el

%.elc: %.el
@printf "Compiling $<\n"
@$(EMACS) -Q --batch $(EMACS_ARGS) $(LOAD_PATH) -f batch-byte-compile $<

check-declare:
@printf " Checking function declarations\n"
@$(EMACS) -Q --batch $(EMACS_ARGS) $(LOAD_PATH) \
--eval "(check-declare-directory default-directory)"

CLEAN = $(ELCS) $(PKG)-autoloads.el

clean:
@printf " Cleaning...\n"
@rm -rf $(CLEAN)

$(PKG)-autoloads.el: $(ELS)
@printf " Creating $@\n"
@$(EMACS) -Q --batch -l autoload -l cl-lib --eval "\
(let ((file (expand-file-name \"$@\"))\
(autoload-timestamps nil) \
(backup-inhibited t)\
(version-control 'never)\
(coding-system-for-write 'utf-8-emacs-unix))\
(write-region (autoload-rubric file \"package\" nil) nil file nil 'silent)\
(cl-letf (((symbol-function 'progress-reporter-do-update) (lambda (&rest _)))\
((symbol-function 'progress-reporter-done) (lambda (_))))\
(let ((generated-autoload-file file))\
(update-directory-autoloads default-directory))))" \
2>&1 | sed "/^Package autoload is deprecated$$/d"
35 changes: 19 additions & 16 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,44 @@
# Maintainer needed ! #

Both, the original creator and the current maintainer have lost the incentive for develop/maintain the package. We even don't have an access to Perforce server any more, so no more testing is done. If you are interested in becoming a maintainer please, [create an issue](https://github.com/qoocku/magit-p4/issues/new?title=I%20want%20to%20be%20a%20maintainer!&body=I%20want%20to%20support%20magit-p4%20and%20have%20access%20to%20Perforce%20server.%0A%0A@lexa)

# Git-p4 plugin for Magit #

[![MELPA](http://melpa.org/packages/magit-p4-badge.svg)](http://melpa.org/#/magit-p4)
[![Build Status](https://travis-ci.org/lexa/magit-p4.svg?branch=master)](https://travis-ci.org/lexa/magit-p4)

Attention! This is in development. Some of the info below is nothing but a plain lie!
This package is in development, and some functionality might be missing.

## Requirements ##

Emacs 24.x with `magit-2.1` or newer and `p4` packages installed. You do not have to install these packages if you use Melpa service - all required dependencies will be downloaded altogether with `magit-p4`.
`git-p4` add-on is obviously required.
Emacs 25.1 with `magit-3.3.0` or newer, `transient-0.4.3` or newer, and `p4` packages installed. You do not have to install these packages yourself if you use [MELPA](https://melpa.org) - all required dependencies will be downloaded together with `magit-p4`.

Additionally, the `git-p4` add-on is required to be installed on your system (outside of Emacs). If running `git p4` gives you a short help string rather than complaining that 'p4' is not a git command, you have it installed and are good to go. Ordinarily, git comes with `git-p4` included, so no extra steps should be necesssary, but Debian and Ubuntu notably do _not_ package it. If that is the case for you, you can download [`git-p4.py` from git's own source](https://github.com/git/git/blob/master/git-p4.py) and put it somewhere in your `$PATH`.

## Installation ##

The package should be available via Melpa service using elpa packages mechanism.
If you plan to enable the package manually be sure to have `magit` and `p4` installed already
and add the downloaded package directory to Emacs `load-path`:
The recommended way to install `magit-p4` is via MELPA and `use-package`:

(add-to-list 'load-path "<path-to-magit-p4-directory")
```elisp
(use-package magit-p4)
```

and then you should call:
If you plan to enable the package manually be sure to have `magit` and `p4` installed already and add the downloaded package directory to Emacs `load-path`:

(add-to-list 'load-path "<path-to-magit-p4-directory>")

and then call:

(require 'magit-p4)

Evalutation of the package code is sufficient to plug into `magit` menu some helpful key bindings.
Regardless of which method you choose, `magit-p4` will automatically hook into magit's menu system and will be available as its own set of commands.

## Usage ##

Being inside Magit status buffer one should press `?` key to reveal magit menu. The menu should be equipped with <kbd>4</kbd> key binded to `magit-p4` submenu, all right. The submenu has four items which represent four activities implemented by `git-p4` add-on:
Inside the Magit status buffer, press <kbd>?</kbd> key to reveal main magit menu. The menu should be equipped with <kbd>4</kbd> key binded to `magit-p4` submenu. The submenu has four items which represent four activities implemented by `git-p4` add-on:

* Cloning - bound to key <kbd>`c`</kbd> and `magit-p4-clone` function;
* Syncing - bound to key <kbd>`s`</kbd> and `magit-p4-sync` function;
* Syncing - bound to key <kbd>`f`</kbd> (similar to magit's fetch) and `magit-p4-sync` function;
* Rebasing - bound to key <kbd>`r`</kbd> and `magit-p4-rebase` function;
* Submitting - bound to key <kbd>`S`</kbd> (capital "s") and `magit-p4-submit` function.
* Submitting - bound to key <kbd>`P`</kbd> (capital "P", similar to magit's push) and `magit-p4-submit` function.

As a shorthand, the <kbd>4</kbd> key is also bound directly in the magit status buffer, similar to the majority of git commands, meaning you don't need to press <kbd>?</kbd> first to access it.

After one of the activities is chosen another menu appears where one may specify additional options and arguments - like `magit` does.

Expand Down
169 changes: 94 additions & 75 deletions magit-p4.el
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
;;; magit-p4.el --- git-p4 plug-in for Magit
;;; magit-p4.el --- git-p4 plug-in for Magit -*- lexical-binding:t; coding:utf-8 -*-

;; Copyright (C) 2014 Damian T. Dobroczyński
;;
;; Author: Damian T. Dobroczyński <[email protected]>
;; Maintainer: Aleksey Fedotov <[email protected]>
;; Package-Requires: ((emacs "25.1") (magit "2.1") (magit-popup "2.1") (p4 "12.0") (cl-lib "0.5"))
;; Package-Requires: ((emacs "25.1") (magit "4.3.0") (transient "0.8.4") (p4 "12.0") (cl-lib "0.5"))
Copy link
Author

Choose a reason for hiding this comment

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

Is there a reason you've set the requirements to the absolutely latest and greatest version for magit and transient? I don't like aggressively recent requirements without a compelling argument, I find it unkind to the users to say that they must have the version that just got released if the code would work just fine with a much older requirement.

The versions I originally picked (3.3.0 and 0.4.3) roughly correspond to the oldest versions I have lying around on any of my machines and which I could test; is there anything wrong with those? If the code remains compatible with the latest versions while targetting old ones, I'd prefer to keep the oldest versions possible as the requirement.

Copy link
Member

@tarsius tarsius Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a major change and to me it makes sense to take the opportunity to make a clear cut.

Supporting very old versions has a very real cost, when done properly (i.e., not just claiming that old versions are supported when in fact it is unknown whether they are), and there are other areas where a maintainer's time is better invested. Stopping to support some old version is more difficult than to not support some old version from the get go. This is an existing package, but its a major upgrade so it is as close to "from the get to" as we can get here.

I do not know what the actual minimally required versions are. Figuring that out would be a massive time investment. I think it is unkind to the user to randomly pick some random version (e.g., "the oldest versions I have lying around"). They might end up sticking to what they have installed, assuming that the specified minimal versions are actually accurate, and then run into issues because the specified required versions were made up.

By requiring users to use a recent version, you force them to make the minimal time investment necessary to update. As an additional benefit they get a ton of fixed bugs (and sure, likely also some regressions).

Magit and Transient have many users, and supporting them takes a lot of time. I am barely able to keep up. So when someone reports a bug, I expect them to help me help them, and that involves updating to the latest releases. You would be surprised how often users report issues that have been fixed long ago. By "forcing" users to use a recent version up front, I save them from getting an initial response of "please update" when they report an issue, and then having to wait for me to get around to look at the report a second time.

Declaring that some older versions are supported, only helps users if the maintainer actually tests that this is still true (regularly, ideally automated). For my major packages I do that when it comes to the Emacs dependency. The cost of supporting old Emacs releases is large. It prevents the use of newer Emacs features and forces the use of workarounds for Emacs bugs that have been fixed long ago. I am not doing that for dependencies that are individual package, not even (or especially) my own packages.

Magit has many users but only one maintainer. I am just human not RHEL. If someone wants RHEL-like stability, they can pay my accordingly.

Magit 3.3.0 is 3.5 years old. It is not reasonable to still be using that. Users who get Emacs package with their distributions package manager, should question whether that is a reasonable thing to do, if it turns out that their distribution is stuck on such an old version.

Transient 0.4.3 has very serious issues. "Emacs accepts no input and has to be killed from the outside" kinda serious.

A reasonable compromise would be to require 4.0.0 and 0.8.0. If a user used these versions and reported an issue, I would still first ask them to update, but at least it is quite possibly safe to assume, that this package works with these versions.

You also have to bump the Emacs dependency. Magit 4.0.0 requires 26.1. Magit >= 4.2.0 requires 27.1. For that reason it is actually a good idea to only require 4.0.0, not 4.3.0, here for a while.

;; Keywords: vc tools
;; URL: https://github.com/qoocku/magit-p4
;; Package: magit-p4
Expand All @@ -30,14 +30,10 @@
;;; Code:

(require 'magit)
(require 'p4)

(eval-when-compile
(require 'cl-lib)
(require 'find-lisp)
(require 'p4)
(require 'subr-x))

(declare-function find-lisp-find-files-internal 'find-lisp)
(eval-when-compile (require 'cl-lib))
(eval-when-compile (require 'subr-x))

;;; Options

Expand All @@ -57,13 +53,13 @@ argument is directory which will hold the Git repository."
(append (list (p4-read-arg-string "Depot path: " "//" 'filespec))
(if (and (not (cl-some (lambda (arg)
(string-match-p "--destination=" arg))
(magit-p4-clone-arguments)))
(transient-args #'magit-p4-clone-popup)))
current-prefix-arg)
(read-directory-name "Target directory: ")
nil)))
(magit-run-git-async "p4" "clone"
(cons depot-path (magit-p4-clone-arguments))))

(cons depot-path (transient-args #'magit-p4-clone-popup))
target-dir))

;;;###autoload
(defun magit-p4-sync (&optional depot-path)
Expand All @@ -77,8 +73,8 @@ depot path which has been cloned to before."
(list (p4-read-arg-string "With (another) depot path: " "//" 'filespec))))
(magit-run-git-async "p4" "sync"
(cond (depot-path
(cons depot-path (magit-p4-sync-arguments)))
(t (magit-p4-sync-arguments)))))
(cons depot-path (transient-args #'magit-p4-sync-popup)))
(t (transient-args #'magit-p4-sync-popup)))))

;;;###autoload
(defun magit-p4-rebase ()
Expand All @@ -90,14 +86,22 @@ depot path which has been cloned to before."
(defun magit-p4-submit ()
"Run git-p4 submit."
(interactive)
(magit-p4-run-git-with-editor "p4" "submit" (magit-p4-submit-arguments)))
(magit-p4-run-git-with-editor "p4" "submit" (transient-args #'magit-p4-submit-popup)))

(defcustom magit-p4-process-yes-or-no-prompt-regexp
"\\[\\(y\\)\\]es, \\[\\(n\\)\\]o"
"Regexp matching yes-or-no prompt for git-p4."
:group 'magit-p4
:type 'regexp)

(defmacro magit-p4-process-kill-on-abort (process &rest body)
;; This is a copy of the obsolete `magit-process-kill-on-abort'.
(declare (indent 1)
(debug (form body)))
`(let ((minibuffer-local-map
(magit-process-make-keymap ,process minibuffer-local-map)))
,@body))

(defun magit-p4-process-yes-or-no-prompt (process string)
(let ((max-mini-window-height 30)
(beg (string-match magit-p4-process-yes-or-no-prompt-regexp string)))
Expand All @@ -108,7 +112,7 @@ depot path which has been cloned to before."
(concat
(match-string
(if (save-match-data
(magit-process-kill-on-abort process
(magit-p4-process-kill-on-abort process
(yes-or-no-p (substring string 0 beg)))) 1 2)
string)
"\n"))))))
Expand All @@ -127,7 +131,7 @@ depot path which has been cloned to before."
process
(concat
(substring
(magit-process-kill-on-abort process
(magit-p4-process-kill-on-abort process
(magit-completing-read prompt '("skip" "quit") nil t))
0 1)
"\n"))))
Expand All @@ -138,6 +142,10 @@ depot path which has been cloned to before."
(magit-p4-process-yes-or-no-prompt process string)
(magit-p4-process-skip-or-quit process string)))

(defun magit-p4--make-reader (function)
(lambda (prompt initial-input _history)
(funcall function prompt initial-input)))

;;;###autoload
(defun magit-p4-run-git-with-editor (&rest args)
"Run git with P4EDITOR set and `magit-p4-process-filter'.
Expand Down Expand Up @@ -171,76 +179,87 @@ P4EDITOR and use custom process filter `magit-p4-process-filter'."
;;; Keymaps

;;;###autoload (autoload 'magit-p4-popup "magit-p4" nil t)
(magit-define-popup magit-p4-popup
(transient-define-prefix magit-p4-popup ()
"Show popup buffer featuring git p4 commands."
'magit-commands
:man-page "git-p4"
:actions '((?c "Clone" magit-p4-clone-popup)
(?s "Sync" magit-p4-sync-popup)
(?r "Rebase" magit-p4-rebase)
(?S "Submit" magit-p4-submit-popup)))

(defvar magit-p4-sync-clone-shared-options
'((?b "Branch" "--branch=")
(?c "Changes files" "--changesfile=" read-file-name)
(?m "Limit the number of imported changes" "--max-changes=")
(?s "Internal block size to use when iteratively calling p4 changes"
"--changes-block-size=")
(?/ "Exclude depot path" "-/")))

(defvar magit-p4-sync-clone-shared-switches
'((?d "Detect branches" "--detect-branches")
(?l "Query p4 for labels" "--detect-labels")
(?b "Import labels" "--import-labels")
(?i "Import into refs/heads/ , not refs/remotes" "--import-local")
(?p "Keep entire BRANCH/DIR/SUBDIR prefix during import" "--keep-path")
(?s "Only sync files that are included in the p4 Client Spec"
"--use-client-spec")))

(magit-define-popup magit-p4-sync-popup
["Actions"
("c" "Clone" magit-p4-clone-popup)
("f" "Sync" magit-p4-sync-popup)
("r" "Rebase" magit-p4-rebase)
("P" "Submit" magit-p4-submit-popup)])

(eval-and-compile ;magit-p4-sync-clone-shared-arguments
(defvar magit-p4-sync-clone-shared-arguments
'(("-b" "Branch" "--branch=")
("-c" "Changes files" "--changesfile=" :reader transient-read-existing-file)
("-m" "Limit the number of imported changes" "--max-changes=")
("-s" "Internal block size to use when iteratively calling p4 changes"
"--changes-block-size=")
("-/" "Exclude depot path" "-/"))))

(eval-and-compile ;magit-p4-sync-clone-shared-options
(defvar magit-p4-sync-clone-shared-options
'(("-d" "Detect branches" "--detect-branches")
("-l" "Query p4 for labels" "--detect-labels")
("-b" "Import labels" "--import-labels")
("-i" "Import into refs/heads/ , not refs/remotes" "--import-local")
("-p" "Keep entire BRANCH/DIR/SUBDIR prefix during import" "--keep-path")
("-s" "Only sync files that are included in the p4 Client Spec"
"--use-client-spec"))))

(transient-define-prefix magit-p4-sync-popup ()
"Pull changes from p4"
'magit-commands
:options magit-p4-sync-clone-shared-options
:switches magit-p4-sync-clone-shared-switches
:actions '((?s "Sync" magit-p4-sync)))

(magit-define-popup magit-p4-submit-popup
["Options"
magit-p4-sync-clone-shared-options]
["Arguments"
magit-p4-sync-clone-shared-arguments]
["Actions"
("p" "Sync" magit-p4-sync)])

(transient-define-prefix magit-p4-submit-popup ()
"Submit changes from git to p4"
:switches '((?M "Detect renames" "-M")
(?v "Be more verbose" "--verbose")
(?u "Preserve user" "--preserve-user")
(?l "Export labels" "--export-labels")
(?n "Dry run" "--dry-run")
(?p "Prepare P4 only" "--prepare-p4-only"))
:options '((?o "Origin" "--origin=" magit-read-branch-or-commit)
(?b "Sync with branch after submission"
"--branch=" magit-read-branch)
(?N "Name of git branch to submit"
" " magit-read-branch-or-commit)
(?c "Conflict resolution (ask|skip|quit)" "--conflict="
(lambda (prompt &optional default)
(magit-completing-read prompt '("ask" "skip" "quit")
nil nil nil nil default))))
:actions '((?s "Submit all" magit-p4-submit)))

(magit-define-popup magit-p4-clone-popup
["Options"
("-M" "Detect renames" "-M")
("-v" "Be more verbose" "--verbose")
("-u" "Preserve user" "--preserve-user")
("-l" "Export labels" "--export-labels")
("-n" "Dry run" "--dry-run")
("-p" "Prepare P4 only" "--prepare-p4-only")]

["Arguments"
("-o" "Origin" "--origin=" :reader ,(magit-p4--make-reader 'magit-read-branch-or-commit))
("-b" "Sync with branch after submission"
"--branch=" magit-read-branch)
("-N" "Name of git branch to submit"
" " :reader ,(magit-p4--make-reader 'magit-p4-read-branch-or-commit))
("-c" "Conflict resolution (ask|skip|quit)" "--conflict="
:choices ("ask" "skip" "quit"))]
["Submit"
("p" "Submit all" magit-p4-submit)])

(transient-define-prefix magit-p4-clone-popup ()
"Clone repository from p4"
:switches (append '((?b "Bare clone" "--bare"))
magit-p4-sync-clone-shared-switches)
:options (append '((?d "Destination directory"
"--destination=" read-directory-name))
magit-p4-sync-clone-shared-options)
:actions '((?c "Clone" magit-p4-clone)))
["Options"
magit-p4-sync-clone-shared-options
("-b" "Bare clone" "--bare")]
["Arguments"
magit-p4-sync-clone-shared-arguments
("-D" "Destination directory" "--destination=" read-directory-name)]
["Actions"
("c" "Clone" magit-p4-clone)])

(transient-insert-suffix 'magit-dispatch '(0 0 0)
'("4" "Git P4" magit-p4-popup))

(magit-define-popup-action 'magit-dispatch-popup ?4 "Git P4" 'magit-p4-popup ?!)
(define-key magit-mode-map (kbd "4") #'magit-p4-popup)

(defun magit-p4/insert-job (&optional job)
"Insert JOB reference in a buffer.

The insertion assumes that it should be 'Jobs:' entry in the
The insertion assumes that it should be `Jobs:' entry in the
buffer. If not - it inserts such at the current point of the
buffer. Then it asks (if applied interactively) for a job id
using `p4` completion function. Finally it inserts the id under
using `p4' completion function. Finally it inserts the id under
`Jobs:' entry."
(interactive
(list (p4-read-arg-string "Job: " "" 'job)))
Expand Down