Re: master b525f201ba: Allow specifying the color to use in image-elide

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>  ;;;###autoload
> -(defun image-elide (&optional square)
> +(defun image-elide (color &optional square)
>    "Elide a square from the image under point.
>  If SQUARE (interactively, the prefix), elide a square instead of a
> -rectangle from the image."
> -  (interactive "P")
> -  (image-crop square t))
> +rectangle from the image.
> +
> +Interatively, the user will be prompted for the color to use, and
> +defaults to black."
> +  (interactive (list (read-color "Use color: ")
> +                     current-prefix-arg))
> +  (image-crop square (if (string-empty-p color)
> +                         "black" color)))

How about putting the color selection behind a prefix argument instead?

How about also adding a defcustom for the default color (i.e. without a
prefix argument)?

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Wed, 14 Sep 2022 16:52:47 -0700
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> How about also adding a defcustom for the default color (i.e. without a
> prefix argument)?

Shouldn't the default be the background color of the frame?

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Shouldn't the default be the background color of the frame?

Yes, I guess that's as good a default as any.

I think I'll follow Stefan's suggestion to make it prompt with a prefix,
and then move the SQUARE stuff into the function itself -- i.e., hit a
key while cropping to switch to square mode.

Eli Zaretskii <eliz@gnu.org> writes:

> I see the same term used in at least one more such program.  So it
> sounds like "cut" is the term we should use here.

OK, I'll go with `image-cut', and move the key binding to `x'.

Hm...  thinking about this for a couple of seconds, it wouldn't be that
hard to make it do what Gimp does -- that is, put the cut pixels into a
"clipboard"...  But I think this would probably have pretty marginal
utility.  Possibly.

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Jean Louis]

* Lars Ingebrigtsen <larsi@gnus.org> [2022-09-15 10:15]:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Shouldn't the default be the background color of the frame?
> 
> Yes, I guess that's as good a default as any.

If default is background color of the frame then it means that cut out
rectangle is transparent, there shall be no color.

However, using default background for images related to frame is very
bad idea. People use various themes and theme of Emacs is not related
to the image.

By default you should have background color to be white and
customizable.

Follow the established standard that foreground and background for
image editing shall be defined somewhere beforehand just as in Gimp.

> Hm...  thinking about this for a couple of seconds, it wouldn't be
> that hard to make it do what Gimp does -- that is, put the cut
> pixels into a "clipboard"...  But I think this would probably have
> pretty marginal utility.  Possibly.

That is very useful, not marginal, that is what we do with images. We
cut out something in order to save it.

There shall be cut and copy options, not just cut. 

Copy means to select and copy, without removing part of the image.

-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

In support of Richard M. Stallman
https://stallmansupport.org/

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Shouldn't the default be the background color of the frame?
>
> Yes, I guess that's as good a default as any.

I don't think the current Emacs theme will affect what I prefer to use
for eliding.  IOW, I'd rather keep it simple and just stay with "black"
as the default (or, even better, adding a defcustom).

> I think I'll follow Stefan's suggestion to make it prompt with a prefix,
> and then move the SQUARE stuff into the function itself -- i.e., hit a
> key while cropping to switch to square mode.

Sounds good, thank you.

> OK, I'll go with `image-cut', and move the key binding to `x'.

FWIW, I like "image-redact" best here.  That also alludes to how I
imagine one would use this.  The problem I see with "cut" is that it
leads you to then think about how to "paste".

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Thu, 15 Sep 2022 00:28:41 -0700
> Cc: emacs-devel@gnu.org
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > OK, I'll go with `image-cut', and move the key binding to `x'.
> 
> FWIW, I like "image-redact" best here.

"Redact" hints on much more than what we do here.

> The problem I see with "cut" is that it leads you to then think
> about how to "paste".

That would be the natural feature to add, indeed.  And when we do, I
don't think we want to rename the command at that time.

So I still think "cut" is a better term.

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Lars Ingebrigtsen]

Stefan Kangas <stefankangas@gmail.com> writes:

> I don't think the current Emacs theme will affect what I prefer to use
> for eliding.  IOW, I'd rather keep it simple and just stay with "black"
> as the default (or, even better, adding a defcustom).

Yes...  the colour you're aiming for here in the final image is probably
unrelated to what your Emacs frame looks like, so perhaps using the
frame background is even worse than just defaulting to black.

Jean suggested defaulting to transparent, but I don't think that's what
people would want here, either.

>> I think I'll follow Stefan's suggestion to make it prompt with a prefix,
>> and then move the SQUARE stuff into the function itself -- i.e., hit a
>> key while cropping to switch to square mode.
>
> Sounds good, thank you.

Now done.

>> OK, I'll go with `image-cut', and move the key binding to `x'.
>
> FWIW, I like "image-redact" best here.  That also alludes to how I
> imagine one would use this.  The problem I see with "cut" is that it
> leads you to then think about how to "paste".

I went with `image-cut' here -- perhaps we'll implement pasting later,
if somebody finds that useful.

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Jean Louis]

* Lars Ingebrigtsen <larsi@gnus.org> [2022-09-16 12:18]:
> Jean suggested defaulting to transparent, but I don't think that's what
> people would want here, either.

Only because "blankness" was mentioned. Color is not blankness to me.x

-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

In support of Richard M. Stallman
https://stallmansupport.org/

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Stefan Monnier]

>> I don't think the current Emacs theme will affect what I prefer to use
>> for eliding.  IOW, I'd rather keep it simple and just stay with "black"
>> as the default (or, even better, adding a defcustom).
>
> Yes...  the colour you're aiming for here in the final image is probably
> unrelated to what your Emacs frame looks like, so perhaps using the
> frame background is even worse than just defaulting to black.
>
> Jean suggested defaulting to transparent, but I don't think that's what
> people would want here, either.

Sounds like "no defaulting" is the better option, then.


        Stefan

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Sounds like "no defaulting" is the better option, then.

Yes...  but after selecting a "cut" colour once, it's perhaps likely
that they'd want to continue using that?  So perhaps `image-cut-color'
should default to nil, but be a multisession variable that remembers the
last choice.

Re: master b525f201ba: Allow specifying the color to use in image-elide

[Lars Ingebrigtsen]

Stefan Kangas <stefankangas@gmail.com> writes:

> How about putting the color selection behind a prefix argument instead?

The prefix was already taken...  and `RET' gives you black.

> How about also adding a defcustom for the default color (i.e. without a
> prefix argument)?

We could add a defcustom instead of hard-coding black; sure.