On Sun, Jun 10 2018, Klemens Nanni <[email protected]> wrote:
> On Sun, Jun 10, 2018 at 01:14:58PM +0200, Marc Espie wrote:
>> Style: you want to add
>> EDIT_PATCHES ?= Yes
>>
>> as well. (Set it around PATCH_CHECK_ONLY)
>>
>> New variables should always be defined, for consistency.
> Done, cheers.
>
>> > +Unless
>> > +.Ev EDIT_PATCHES
>> > +is set to
>> > +.Sq \&No ,
>> > +changed files are opened using
>> > +.Ev VISUAL ,
>> > +.Ev EDITOR
>> > +or
>> > +.Xr vi 1 .
>> Too many details. That's just the standard way to invoke your editor.
>> Redundant with variable description.
> Fine with me; just wanted to be clear.
>
>> See
>> .Ev EDIT_PATCHES .
>>
>> should be enough
> Done.
No big objection, I would have preferred a fix to allow the user to
press 'n' but I guess it can be done later... Please see below,
> Index: bsd.port.mk
> ===================================================================
> RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
> retrieving revision 1.1414
> diff -u -p -r1.1414 bsd.port.mk
> --- bsd.port.mk 4 Jun 2018 06:14:56 -0000 1.1414
> +++ bsd.port.mk 10 Jun 2018 11:43:47 -0000
> @@ -128,6 +128,7 @@ _ALL_VARIABLES_INDEXED += COMMENT PKGNAM
> .endif
>
> PATCH_CHECK_ONLY ?= No
> +EDIT_PATCHES ?=
Should be
EDIT_PATCHES ?= Yes
> REFETCH ?= false
> PORTROACH ?=
>
> @@ -2362,11 +2363,9 @@ update-patches:
> PATCH_LIST='${PATCH_LIST}' DIFF_ARGS='${DIFF_ARGS}' \
> DISTORIG=${DISTORIG} PATCHORIG=${PATCHORIG} \
> ${_PERLSCRIPT}/update-patches`; \
> - case $$toedit in "");; \
> - *) read i?'edit patches: '; \
> - cd ${PATCHDIR} && $${VISUAL:-$${EDITOR:-/usr/bin/vi}} $$toedit;; esac
> -
> -
> + if [ -n "$$toedit" -a "${EDIT_PATCHES:L}" != no ]; then \
I dislike using -a and -o in classic test/[ commands. Even if we don't
care in bsd.port.mk -a/-o are not standard; also test -a is used in
a single other place. Could you please amend the check like shown below?
if [ -n "$$toedit" ] && [ "${EDIT_PATCHES:L}" != no ]; then
> + cd ${PATCHDIR} && $${VISUAL:-$${EDITOR:-/usr/bin/vi}} $$toedit;
> \
> + fi
>
> .endif # IGNORECMD
>
> Index: bsd.port.mk.5
> ===================================================================
> RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v
> retrieving revision 1.481
> diff -u -p -r1.481 bsd.port.mk.5
> --- bsd.port.mk.5 29 May 2018 11:45:25 -0000 1.481
> +++ bsd.port.mk.5 10 Jun 2018 11:44:04 -0000
> @@ -743,27 +743,9 @@ See
> .Cm lock .
> .It Cm update-patches
> Create or update patches for a port, using
> -.Xr update-patches 1 ,
> -which invokes
> -.Xr diff 1
> -between
> -.Pa file
> -and
> -.Pa file.orig ,
> -based on
> -.Pa file.orig
> -existence.
> -In order to generate a patch, the original file needs to be named
> -.Pa file.orig
> -and
> -.Pa file
> -edited.
> -After the target is invoked, the patches are placed under the
> -patches/ directory.
> -It moves existing patches from
> -.Pa patch-file
> -to
> -.Pa patch-file.orig .
> +.Xr update-patches 1 .
> +See
> +.Ev EDIT_PATCHES .
> .It Cm update
> Update an existing installation to a newer package:
> scan the installation for a package with the same
> @@ -1572,6 +1554,12 @@ to see
> .Ev REORDER_DEPENDENCIES
> actions.
> Silent by default.
> +.It Ev EDIT_PATCHES
> +User settings.
> +If set to
> +.Sq \&No ,
> +.Cm update-patches
> +will not open changed files in an editor.
Nitpicking, what about adding "Defaults to 'Yes'"?
> .It Ev EPOCH
> Epoch number of the current package.
> Defaults to empty (no need for numbering changes), then
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE