bug#29110: 25.2; Should push-mark allow duplicates?

bug#29110: 25.2; Should push-mark allow duplicates?

[Pierre Neidhardt]

The `push-mark' function allows for duplicate marks.  I fail to see a
use case, but otherwise I think it's rather inconvenient:

- It makes traversing the ring tedious with respect to end-user
interaction.  (Think Ivy / Helm for the mark ring.)
Duplicates are probably not the expected behaviour for the end-user.

- Functions working with rings will probably want to remove the
duplicates, so they end up calling `remove' and the like over and over
again.

- It eats up more memory.

- It's counter-intuitive to developers who may in turn write code
without being careful that rings may contain duplicates.  This may
result in unexpected behaviour.


I got bitten hard by this while trying to figure out why
`helm-mark-ring` would randomly fail to follow the marks when Evil mode
was `require'd (not even turned on).

I reported the issues on both bug trackers:

- https://github.com/emacs-evil/evil/issues/845

- https://github.com/emacs-helm/helm/issues/1891

We could not find out the root of the issue, but we discovered that
advising `push-mark' so that it does not duplicate marks would do it.  I
know it's not a solution per se, but at least we've got a lead.

To reproduce, start emacs -Q and use "C-SPC C-SPC" a few times at
different spots.  Then move point somewhere and eval:

	(set-marker (mark-marker) (point))
	(push-mark)
	mark-ring

You should now have one duplicate in the ring.

Here is the proposed fix implemented in Helm:

https://github.com/emacs-helm/helm/commit/ffd2abf5c4bdfc998c09730387b11d2bf9ac1032




In GNU Emacs 25.2.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.16)
 of 2017-09-02 built on dhiov23k
Windowing system distributor 'The X.Org Foundation', version 11.0.11905000
System Description:	Gentoo Base System release 2.4.1

Configured using:
 'configure --prefix=/usr --build=x86_64-pc-linux-gnu
 --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
 --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
 --localstatedir=/var/lib --disable-dependency-tracking
 --disable-silent-rules --docdir=/usr/share/doc/emacs-25.2
 --htmldir=/usr/share/doc/emacs-25.2/html --libdir=/usr/lib64
 --program-suffix=-emacs-25 --infodir=/usr/share/info/emacs-25
 --localstatedir=/var
 --enable-locallisppath=/etc/emacs:/usr/share/emacs/site-lisp
 --with-gameuser=:gamestat --without-compress-install
 --with-file-notification=inotify --enable-acl --without-dbus
 --without-modules --without-gpm --without-hesiod --without-kerberos
 --without-kerberos5 --with-xml2 --without-selinux --with-gnutls
 --without-wide-int --with-zlib --with-sound=alsa --with-x --without-ns
 --without-gconf --without-gsettings --without-toolkit-scroll-bars
 --with-gif --with-jpeg --with-png --with-rsvg --with-tiff --with-xpm
 --with-imagemagick --with-xft --without-cairo --without-libotf
 --without-m17n-flt --with-x-toolkit=gtk3 --without-xwidgets
 GENTOO_PACKAGE=app-editors/emacs-25.2 'CFLAGS=-march=ivybridge -O2
 -pipe' CPPFLAGS= 'LDFLAGS=-Wl,-O1 -Wl,--as-needed''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND NOTIFY ACL GNUTLS LIBXML2
FREETYPE XFT ZLIB GTK3 X11

Important settings:
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: Debbugs

bug#29110: 25.2; Should push-mark allow duplicates?

[Drew Adams]

> The `push-mark' function allows for duplicate marks.  I fail to see a
> use case, but otherwise I think it's rather inconvenient:

It's not necessarily inconvenient for everyone or every use case.

If you or Helm or whatever wants to remove duplicates you can
always do so.  You can also prevent a duplicate from being
added.  And you can choose whether pushing what would otherwise
become a duplicate should remove an "older" duplicate member or
prevent the "newer" potentially duplicate member.  There are
lots of possibilities.

Consider the simple case of someone using vanilla `C-u C-SPC'.
Someone might want to have duplicates at different positions
in the ring, visiting them in order.

I, like you apparently, use an interface that lets me cycle
among marks in various ways (various orders), cycle among
various subsets of the available marks (filtering), or access
marks directly.  And with the interface I use (Icicles) I
can choose to remove duplicates, in general.  (And the default
command that moves among markers does remove duplicates.)

But I can imagine other interfaces and other use cases.

I think this kind of choice should be up to the user or the
interface author.  Helm can decide to remove/prevent duplicates,
or you can as one user.  I don't see why Emacs itself should.
(Just one opinion, though.)

> - It makes traversing the ring tedious with respect to end-user
> interaction.  (Think Ivy / Helm for the mark ring.)
> Duplicates are probably not the expected behaviour for the end-user.

It depends on the interface, the user, and the use case.

> - Functions working with rings will probably want to remove the
> duplicates, so they end up calling `remove' and the like over and over
> again.

Why "over and over again"?  You can prevent adding duplicates, no?

> - It eats up more memory.

Seriously?  Bof.

> - It's counter-intuitive to developers who may in turn write code
> without being careful that rings may contain duplicates.  This may
> result in unexpected behaviour.

Bof.  Developers should get beyond depending on any such naive
"intuition".

> I got bitten hard by this

And now you know. ;-)

> We could not find out the root of the issue, but we discovered that
> advising `push-mark' so that it does not duplicate marks would do it.  I
> know it's not a solution per se, but at least we've got a lead.

It's a fine individual solution, IMO, if you never want duplicates.

bug#29110: 25.2; Should push-mark allow duplicates?

[Pierre Neidhardt]

I understand there might be some use cases, I just don't see which one.



> Consider the simple case of someone using vanilla `C-u C-SPC'.
> Someone might want to have duplicates at different positions
> in the ring, visiting them in order.

Hmm, I'm tempted to say "bof" too here :)  You are not saying _why_ the
user would like to see duplicates.



>> - Functions working with rings will probably want to remove the
>> duplicates, so they end up calling `remove' and the like over and over
>> again.
>
> Why "over and over again"?  You can prevent adding duplicates, no?

In every function calling push-mark, yes.  That's what I mean with "over
and over again".  How do you prevent this without advising push-mark?



>> - It eats up more memory.
>
> Seriously?  Bof.

Indeed, bof, that was more of a wink.  I do not know the Emacs standard
in memory consumption though.

But consider this: with Evil, jumping between two marks (so just
navigating between them) will duplicate each mark every time.  You might
argue that this is bad code on Evil's side.  But then high-level
functions might call the jumping functions in loops...  And so on.

The point is that for somebody writing some fairly high level code, it
gets quite obscur as to why the mark-ring blows up.

I know, none of this is a valid reason.



Regardless, I realize that I failed to formulate a proper query in my
initial report: Does anybody have a hunch as for why duplicate marks
could potentially interfere with code manipulating the mark-ring?  See
the issues on Helm and Evil.

--
Pierre Neidhardt

bug#29110: 25.2; Should push-mark allow duplicates?

[Noam Postavsky]

Pierre Neidhardt <ambrevar@gmail.com> writes:

>>> - It eats up more memory.
>>
>> Seriously?  Bof.
>
> Indeed, bof, that was more of a wink.  I do not know the Emacs standard
> in memory consumption though.
>
> But consider this: with Evil, jumping between two marks (so just
> navigating between them) will duplicate each mark every time.  You might
> argue that this is bad code on Evil's side.  But then high-level
> functions might call the jumping functions in loops...  And so on.

Consider this:

    mark-ring-max is a variable defined in ‘simple.el’.
    Its value is 16

    Documentation:
    Maximum size of mark ring.  Start discarding off end if gets this big.

    You can customize this variable.

global-mark-ring-max is the same idea.

> Regardless, I realize that I failed to formulate a proper query in my
> initial report: Does anybody have a hunch as for why duplicate marks
> could potentially interfere with code manipulating the mark-ring?  See
> the issues on Helm and Evil.

Both threads seem pretty hard to follow.  One thing to note is that
push-mark resets markers before discarding (so that they no longer point
to any buffer).  Maybe Helm or Evil keep another reference to a
discarded marker, and try to use it without checking?  If you have a way
to reproduce the problem, you can check if bumping up mark-ring-max to a
very large number has any effect.

    (defun push-mark (&optional location nomsg activate)
       ...
        (when (> (length mark-ring) mark-ring-max)
          (move-marker (car (nthcdr mark-ring-max mark-ring)) nil)
          (setcdr (nthcdr (1- mark-ring-max) mark-ring) nil)))
       ...
        (when (> (length global-mark-ring) global-mark-ring-max)
          (move-marker (car (nthcdr global-mark-ring-max global-mark-ring)) nil)
          (setcdr (nthcdr (1- global-mark-ring-max) global-mark-ring) nil)))

bug#29110: 25.2; Should push-mark allow duplicates?

[Pierre Neidhardt]

> Consider this:
>
>     mark-ring-max is a variable defined in ‘simple.el’.
>     Its value is 16
>
>     Documentation:
>     Maximum size of mark ring.  Start discarding off end if gets this big.
>
>     You can customize this variable.
>
> global-mark-ring-max is the same idea.

Damn, I had no clue about that one.  Thank you for shattering my
ignorant concern to pieces! :D

That being said, it raises another problem (sorry for being so
annoying): if we have a limit _and_ we allow duplicates, it means that a
lot of duplicates will discard old entries more quickly.

> One thing to note is that push-mark resets markers before discarding
> (so that they no longer point to any buffer).

Thanks a lot, that's an excellent lead.  Indeed, that might be the root
of the issue.  We will investigate further.

Thanks for all!

--
Pierre Neidhardt

Here is a test to find whether your mission on earth is finished:
if you're alive, it isn't.

bug#29110: 25.2; Should push-mark allow duplicates?

[Drew Adams]

> I understand there might be some use cases, I just don't see which one.
> 
> > Consider the simple case of someone using vanilla `C-u C-SPC'.
> > Someone might want to have duplicates at different positions
> > in the ring, visiting them in order.
> 
> Hmm, I'm tempted to say "bof" too here :)  You are not saying 
>_why_ the user would like to see duplicates.

What part of "visiting them in order" wasn't clear?

That's the point of having a sequence (or ring): order
and (possibly) duplicates.

> >> - Functions working with rings will probably want to remove the
> >> duplicates, so they end up calling `remove' and the like over and over
> >> again.
> >
> > Why "over and over again"?  You can prevent adding duplicates, no?
> 
> In every function calling push-mark, yes.  That's what I mean with "over
> and over again".  How do you prevent this without advising push-mark?

Advising `push-mark' is fine, if that's what you always want,
everywhere.  If not, define your own `my-push-mark` for the
use cases you have.

> But consider this: with Evil, jumping between two marks (so just
> navigating between them) will duplicate each mark every time.

Then there is something wrong with Evil, perhaps.
With vanilla Emacs, jumping to a marker does not push
another marker there.

> You might argue that this is bad code on Evil's side.

I might, if I understood it more.  I really have no idea.

> But then high-level functions might call the jumping
> functions in loops...  And so on.

And so on sounds like compounding a problem.  Is it a problem
of Evil's own making?  Why would it push a mark each time it
jumps to a marker.  Vanilla Emacs doesn't do that.

bug#29110: 25.2; Should push-mark allow duplicates?

[Pierre Neidhardt]

> What part of "visiting them in order" wasn't clear?

Why visiting in order?
I understand why rings should preserve the order in general, but what is
the user's intention when visiting marks in order?  Why does order
matter in this specific case?  Why going A-B-A?

Maybe the problem is that I see marks as "bookmarked position in text", and in
this case it makes little sense to have a bookmark order.

-- 
Pierre Neidhardt

Preserve Wildlife!  Throw a party today!

bug#29110: 25.2; Should push-mark allow duplicates?

[Drew Adams]

> > What part of "visiting them in order" wasn't clear?
> 
> Why visiting in order?

Why not?  You asked for a possible use case.  To me that
is one.  You mark spots to visit, then you cycle among
them.  The order of marking determines the (default)
order of cycling.  Duplicates determine when and how
often you visit a particular node in the cycle.

The point is this: If Emacs automatically always removes
duplicates then, well, you cannot take advantage of
anything that duplicates can give you.  But if Emacs
does not remove duplicates then you can - and you can
always remove/prevent duplicates.

A sequence with duplicates gives you more possibilities
than a sequence without duplicates, simply because you
can always remove duplicates but you cannot easily add
them.  That is, you cannot add them if Emacs keeps
automatically removing/preventing them.  Why impose that?

> I understand why rings should preserve the order in general, but what is
> the user's intention when visiting marks in order?  Why does order
> matter in this specific case?  Why going A-B-A?

T-U-V-a-B-C-D-a-E-F-G-H-I lets you visit `a' more
easily, more often, and quicker.  Whatever.  Whatever
use someone or some code might put duplicates to.  Why
should Emacs prevent such a possibility out of the box,
especially since it is so easy for anyone or any code
to not allow duplicates.

> Maybe the problem is that I see marks as "bookmarked
> position in text", and in this case it makes little
> sense to have a bookmark order.

I too see them that way.  And there is every reason
to have a bookmark order - that can be very useful.

Multiple bookmarks in a buffer, in buffer order,
let you cycle among them in buffer order.  In alpha
order by, say, the thingie definitions that they
mark gives you alpha-order navigation.  Any number
of orders are possible.  Different orders for
different purposes and different users.

Isn't that what Helm & Ivy give you: easy ways to
navigate among the markers in different orders?
If they don't let you easily switch navigation
orders then they should (IMHO).  (In Icicles, at
lease, it it is simple and immediate to change to
a different order.)

_Cycling_ navigation is in fact largely about
_order_.  Put differently: order makes a big
difference if you need to cycle to get to something.

Order can impose inconvenience.  Imagine cycling
through every name in the phone book (remember
phone books?), in alphabetic order starting at A,
just to get to the name Neidhardt.  Not fun,
especially if it is a large phone book.

That's why interfaces such as Icicles, Helm, etc.
also let you _filter_, to reduce the sequence to
cycle through.  And it's why they also let you
go _directly_ to a given element in the
navigation list, e.g., by name.

bug#29110: 25.2; Should push-mark allow duplicates?

[Pierre Neidhardt]

>> Maybe the problem is that I see marks as "bookmarked
>> position in text", and in this case it makes little
>> sense to have a bookmark order.
>
> I too see them that way.  And there is every reason
> to have a bookmark order - that can be very useful.

I'm still skeptical about that one.  That being said, that's not a
reason to remove the feature, you are right.

Enough bike-shedding :)

--
Pierre Neidhardt

To err is human -- to blame it on a computer is even more so.

bug#29110: 25.2; Should push-mark allow duplicates?

[Drew Adams]

> >> Maybe the problem is that I see marks as "bookmarked
> >> position in text", and in this case it makes little
> >> sense to have a bookmark order.
> >
> > I too see them that way.  And there is every reason
> > to have a bookmark order - that can be very useful.
> 
> I'm still skeptical about that one.  That being said,
> that's not a reason to remove the feature, you are right.

Yes, you don't need to be convinced, as long as the
proposal is abandoned.

But FWIW, cycling order can be quite important for
users, particularly for a situation, such as with
vanilla Emacs cycling, where you have _no alternative_,
where you cannot act on candidates (in this case visit
bookmarks/marks) _selectively_, using direct access.

For bookmarks - and the same goes for Emacs markers,
two cycle orders that can be _particularly_ useful
are: (1) order on the page, i.e., in the buffer, and
(2) time order: of creation or last modification or
last access.

If you don't get that then I can only guess that you
don't cycle among such marks very often.  Or you might
not get it if you use something (e.g. Helm?) that _does_
let you access candidates other than only by cycling,
drone-like, through a huge list of candidates, in order.

Another possibility is that the lists that you cycle
through are relatively short.  In that case, the order
has little or no importance.

For example, with vanilla Emacs Info `i' (`Info-index'), 
it's not very bothersome that one can only cycle through
multiple matches in a single, predefined order, using `,',
because there are only a very few candidates (e.g. less
than 5) to cycle among.

Being limited to cycling becomes problematic when there
are many candidates to choose from.  UIs such as Icicles
and Helm let you pare down the set of matching candidates,
incrementally, and they (at least Icicles does) also let
you access any number of them directly (e.g. click or hit
a key).

> Enough bike-shedding :)

This isn't bike-shedding.  It's more like discussing
a proposal to remove the kitchen altogether because
some users always eat out and no longer cook at home. 

The epithet "bike-shedding" can be, BTW, a facile
put-down, tossed out when one has nothing useful to
add further to a discussion.

It dismisses the discussion itself as unimportant,
as a substitute for acknowledging a weak or abandoned
argument.  It confuses personally wanting to drop a
discussion with the relative importance of the
discussion.

Whether you really care about your proposal is not
so relevant, once it has been dropped, as whether it
raises an important or useful question, i.e., whether
the discussion of the proposal serves a purpose.

The question of duplicate candidates, and more
generally of cycling, and even more generally of how
to choose and access candidates, is an important one.

Removing duplicates _can_ be very useful.  What we
should avoid, IMO, is removing duplicates willy nilly,
or in all cases.  Less generally, systematically 
removing duplicates for marker navigaion would be a
mistake, IMO.

But this general question is worth considering: How
can Emacs provide users with the ability to remove
duplicates when they want to, either on the fly (e.g.
hit a key) or by configuration for a given feature.

I don't think that vanilla Emacs has a good answer for
that question.  Icicles (and I assume other UIs that
have since adopted a similar approach) lets users hit
`C-$' to toggle the removal of duplicate candidates on
the fly.  You can also configure a given command, to
have it remove duplicates from the get-go.

Vanilla Emacs provides no on-the-fly user modification
of the set of candidates or their order of access.
That's really the rub scratched by the question in the
Subject line here.  A bug thread is not the main place
to discuss such a problem thoroughly, but it can be a
place to raise the question.

bug#29110: 25.2; Should push-mark allow duplicates?

[Lars Ingebrigtsen]

Pierre Neidhardt <ambrevar@gmail.com> writes:

> The `push-mark' function allows for duplicate marks.  I fail to see a
> use case, but otherwise I think it's rather inconvenient:

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

I think the argument for not filtering duplicates is that it makes
traversing the history less predictable -- if you do two point-setting
commands, you know that you have to pop the stack twice to get back to
before that.

And we can't change the default of such a basic command, because it'd
annoy too many people.  (If somebody wants push-mark to filter
duplicates, it'd be easy to add advice to push-mark to do that.)

So I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no