Suggestion for a new configuration interface

[ilohmar]

DISCLAIMER: This is a bold suggestion, and so most likely a stupid idea. But Fuco1 encouraged me to submit any ideas I have, so, mkay... The method looks quite different from the current interface. Worse, I do not understand the internals well enough to know if this is feasible at all, so it might come across as all-talk, know-it-all, patronizing, grandiloquent---sorry, not my intention for sure. It is not too hard to configure the wonderful smartparens package as it stands, but maybe it could be even easier and/or cleaner.

Here is an example of many features and the interface that I have in mind:

;; add some global pairs:
(sp-pair "\"" "\""
         :actions '(wrap insert) ;default: wrapping and inserting pairs
         :inhibit nil)           ;default: never inhibit, accepts arb fn
(sp-pair "`" "'")
(sp-pair "'" "'"
         :actions nil)           ;as if pair was never defined at all..
;; global /tags/ are hard to imagine, so we do not have a command.
;; local pairs override global behavior /or/ define additional pairs:
(sp-local-pair 'org-mode
               "_" "_"           ;not in global pairs yet!
               :actions '(wrap insert)
               :inhibit #'in-src-block-p) ;applies to inserting only
(sp-local-pair 'org-mode
               "*" "*"
               :actions '(insert) ;override: no wrapping (bad example)
               :inhibit #'org-at-heading-p)
(sp-local-pair 'weird-mode
               "\"" "\""
               :actions nil)     ;fully restores non-sp behavior
(sp-local-pair '(lisp-mode scheme-mode) ;list of modes still works
               "`" "'"
               :inhibit #'in-code-p) ;override: insert only in strings/comments
;; local tags:
(sp-local-tags 'latex-mode
               "\\b" "\\begin{_}\n" "\\end{_}\n"
               'identity                ;the tag transformation
               :actions '(wrap insert)  ;both make sense for tag pairs as well
               :post-handler #'my-latex-reindent)

Advantages as I see them:

• most options, incl. sp-ignore-modes-list, work as before
• clear distinction between pairs and tags, and between wrapping and
  insertion
  • can have any combination of actions
• no need for additional remove or ban functions---just set actions
  argument to nil
  • Ex: to have a pair work almost everywhere, add it globally, then
    override locally with ":action nil"
  • or override inhibit argument for given mode(s)
• best feature imho: inhibit argument (with some pre-defined predicate
  functions) unifies "in-code|string" tests with
  sp-autoinsert-inhibit-functions (for user-defined predicates) ---
  those usually apply to a particular pair only anyway, I guess
  • for starters the predicate gets the same argument as now
• note that together, this cuts the number of functions to configure
  all pairing-type options down to three, and at the same time, the
  logic can be seen more easily
• optionally (easy to extend as well! from a UI point, that is)
  • pairs could be lists of conses: do the same thing for several
    pairs
  • add keywords for auto-escape and repeated-wrapping behavior
  • or other things like the fictional :post-handler above

So there you have it. Tear it to shreds, ignore it, or discuss and find something useful in it. Preferably the latter :)

[Fuco1]

Hey, I've only skimmed it over but it looks pretty good. I don't have time to check it out or reply with something reasonable (exams yay). I'll look at it ASAP! Thanks for suggestions, it's always welcomed.

[Fuco1]

Ok, I've read it all and I like it quite a lot :) The best thing is that you cleverly came up with functions that are not yet present, so we can keep both systems in parallel (at least for a time being until we hit "version 2" or something). If (when!) this gets implemented the old system will become deprecated, and removed after a while.

Not to mention that the internal permissions data structures are quite a mess... that's the downside of "rapid prototyping developement" I guess :P

The post handler is also a good idea. I've actually had that in mind, but I didn't know how exactly to implement it and make available to the user. My original idea was to add lots of "smartparens-mode.el", similar to what expand-region has, but this is better. (we might still add those files if we gather enough functions for some mode that it'd warrant an extra file)

[ilohmar]

Glad you like it! Keeping the systems in parallel might be a good idea. But I also thought that the internals might get modified in the process, and the current system might get harder to maintain. In principle the system suggested above should map nicely 1:1 onto an internal data structure (or that's what I hope). Like a single global default plist of pairs, actions, inhibits etc, and a mode- (rather buffer-)local list of the same structure that overrides or adds to it.

Good luck with the exams!

[Fuco1]

I still think that some sort of 'remove' function would be handy, mostly for the attributes where you can specify lists (inhibit actions, post hooks (pre hooks?)). How to handle that?

Well, I acually think the internals will get modified in the process. I imagine a simple datastructure like this:

("(" ;; opening
 ")" ;; closing
 (
  ((blarg-mode) . "]") 
  ((foo-mode baz-mode) . "}")
 ) ;; list of modes where the end pair is overloaded. From this, a buffer-local simple pair list will be extracted.
 (list of actions)
 (list of inhibit functions)
 (list of pre hooks)
 (list of post hooks)

If you define local pair only, the closing pair will be nil to reflect that. This structure maps 1:1 (almost) to the proposed functions. The current system will be changed into macros that will expand to apropriate calls of the new functions, so we really only have one system. (plus I'll mark them as obsolete and then remove later).

Something similar for tag pairs.

[ilohmar]

I very much like your idea for how to do a transition.

Regarding the data structure, I worry that it cannot cope (as an example) with the case that in a certain mode you might want the same open and close delimiters as globally, but want to disable only wrapping behavior, or an inhibit function just in this mode.

So if it's feasible I would rather keep a global structure without any information on mode-specific things, and keep exactly the same per mode. Both kinds of data can be manipulated separately by the user (via the above functions), and afterwards the functions use the global and the mode-local list to set up the buffer-local values. This should be very flexible for any future behavior you might want to add. I used plists for this reason of keeping it extensible, but of course a fixed list structure works just as well.

For a remove-type function for the lists, you could define one function per each above that takes (maybe mode) trigger, entry key (specifying what to manipulate), what-to-remove, which just manipulates the list entry. I am not sure when you would really need this, however. For me, configuring smartparens is set-and-forget, and for this, the three interfaces above should suffice, unless I am missing something.

There are some subtleties, but nothing out of the ordinary, for example distinguishing "we want to use the global setting" (maybe as a "t" argument) from "override with nil for this mode" in the user functions.

[Fuco1]

Yea, you're probably right regarding the remove functions. I guess I'm the only person who ever use them, and that's only for the testing.

About the data, yea, I haven't thought of that. Plists are also a good idea since it also makes for easier access to the items (another thing I had no idea when I started with SP). So something like (ignoring the inhibit/handler etc for clarity):

;; t means global, for now.
(
 (:where t :open "`" :close "'" :actions (actions) ...)
 (:where t :open "(" :close ")" :actions (actions) ...)
 (:where '(markdown-mode gfm-mode) :open "`" :close "`" :actions (actions) ...) 
 ;; or should we keep it "one list per mode". This might make it simpler to set things up, otoh introduces a bit of duplication.
 (:where markdown-mode :open "`" :close "`" :actions (actions) ...) 
 (:where gfm-mode :open "`" :close "`" :actions (actions) ...) 
)

But then, how to query such a thing? Most likely I'd need to access pairs based on the opening pair and by modes (to set up local versions). So it'd require two alists, or some functions that would work with these plists---which I guess is not so difficult, if only it would make it a bit less efficient. But until the lists are thousands of items long I don't think that matters much.

Or, if you have a different idea, please give me a concrete example :P It's early in the morning and the coffee isn't kicking in yet.

[ilohmar]

I would keep a single big alist with entries per mode or global,

((t . global-entry-here)
 (org-mode . another-entry)
 ...)

Easily queried with assq, and that only for setting up the buffer-local settings. Now each global/mode-entry is another alist/plist (I do not remember if this structure will work ok..)

(("`" :close "'" :actions (foo bar) ...)
 ("(" :close ")" :actions ...)
 ...)

Again easy queries. And this is exactly what the buffer-local entry would look like (structure-wise), only with global values properly merged. This should be very small and fast to use, I suppose.

I am not sure if/how one should merge the tag-insertion/tag-wrapping here as well. Probably not, keeping separate structures of a similar form (slightly different plist keys) will be cleaner. In some cases both would be queried to find out what to do, but that seems very reasonable to me.

[Fuco1]

Yes, now I understand.

No, the structure would not work as I think the plist works by looking for keys on even positions (starting with zero), so it also needs :open tag. Which makes it a bit unconvenient since now you can't use "assq"... but that's not a problem since with dash.el it's super easy to query on second element instead.

I'll write up a lenghtier wiki article with all these changes and some technical details (like, proper description of all the fields etc.), so we can have a more global look at all these ideas properly explained.

I'll get to that maybe around 2AM tomorrow.

One more problem with this, the keyword arguments require CL. Now, I read in the emacs docs that CL is discouraged during runtime. I understand defun* is a macro so that's OK, but I also use 'signum' and 'cadar' style functions in my code. I'm not sure yet how to fix this... maybe I'll include them to the sp namespace. "cadar" should actually be inlined so it should work fine without loading CL (I need to test that more).

[ilohmar]

You could also use a mode (and buffer-local) entry like

(("`" . (:close "'" :actions ...))
 ("(" . (:close ")" :actions ...))
 ...)

to make it an easy alist, looks fine to me. The wiki article is a great idea, BTW. And take some time off in between :)

[Fuco1]

So I'm finally getting back to dev work (exams are killing me :/). I've found another thing I think we should discuss.

• Imagine you have a pair with bunch of settings, and then want to add/remove some of them locally. For example, you have 2 actions on a pair, and you want to remove one of them in some local mode. Now, you can only do it by specifying the new list. But what if we add different action (i.e. a third action besides insert/wrap) later on? Then you'd have to modify both entries, instead of just the default one. With lists of inhibit functions/post-insertion-handlers this can be even more useful (I plan to eventually remove most of the ad-hoc customize stuff and add predicates to be put into inhibit list)

So I'm sort of thinking how to design "pair inheritance" the best way, so you can only specify what to add/remove from the global pair.

My idea so far:

1. Use (t new-local-thing-1, ...) where t signifies "use the global list plus add all on this list"
2. Use (nil remove-this-1, ...) where nil signifies "use the global list and remove this"
3. Use ((t ...) (nil ...)) if you need both 1 and 2.

This looks neat to me, but I don't know how confusing it might get. Please give suggestions.

• Also, how about we actually get rid of sp-local-pair as well and just add :modes '(...) to sp-pair? If we're going for minimal interface, it makes a bit of sense. Altho, maybe the extra function will make it more clear that this is a sort of "special" function that overrides defaults.
• How to handle multiple pair arguments. If I make two first mandatory arguments "open" and "close"? Should I only take cons pairs? Then it can be cons pair or a list of cons pairs. But for one pair only, two separate arguments look better.
• We should also probably change 'inhibit' to something else, as it is a verb now and the rest of arguments are nouns (as they should be). Anyone has an idea for a good name? Also, should we use plurals or singulars? (action/s, post-handler/s...?)

[ilohmar]

I'll start with a comment on the last point: Minimal is good, but here I think a separate sp-local-pair function would better. Otherwise setting global pairs and local additions/removals/overrides just seem to similar, when they really are not. Also it seems natural since they also need (kinda) separate data structures. And handling the overrides the way you suggest also happens only in sp-local-pair.

The idea for overriding by specifying differences looks interesting. If you go this route I would suggest explicit 'add and 'remove symbols instead of t and nil. So one could specify

• nil for an empty list (pair without effect if applied to actions, or no inhibit functions etc)
• a list to override (or set) explicitly
• and the above, just maybe with add and remove
• with (add) or (remove) alone one could inherit global settings
  Yeah I think I really like that! It is lean and explicit and unambiguous.

I agree it is best to make special functions into predicates as far as possible, although I do not use the customize interface.

[Fuco1]

Oh, the add/remove idea is very nice. I'm finishing the wiki article, it will be up any moment.

Yea, (add) can be a "placeholder" for simple inheritance. As such, it will be a default argument so when you don't specify anything, it will simply use the same set.

Here is the wiki article with the interface as it is so far. I'll add the internals tomorrow, I still have to figure out some minor details.

The IMPLEMENTATION NOTES are so far "undecided" or not discussed issues.

By the way, I really appreciate your input. This is going to be so sweet when it's done! :P

[ilohmar]

Just some additional comments on the wiki page:

• how about merging inhibit-wrap with inhibit (also keeping future action additions in mind)? the functions get the action anyway (so they can test for it), and in general one would rarely permit explicit wrapping, I guess
• the inhibit predicates should still get some info about the context (in string, in code), shouldn't they? this makes tests much more straightforward. on the other hand, it is cleaner if users can add their own context tests.. hmm undecided out that one

[Fuco1]

By the way, I've changed "inhibit" to "filters", because it should be a noun in plural like the rest of the arguments.

I've also removed the -wrap variant, you're right that it's cleaner that way.

Yea, it can also get the basic context from SP, that can't hurt. If not needed, you can simply ignore the argument. I also plan to add more contexts, like "in math mode" (for latex) so users don't have to reimplement that each for their own predicates.

I have the management part +/- ready. Now I need to map the old functions to new ones and extract the tests from insert/wrap functions. I hope it will be ready sometime next week.

It wasn't even that much code, just shows it's a pretty good design.

[Fuco1]

This is currently impossible to express: "Only autoinsert this pair in specific mode inside strings". You might for example want this inside java-doc comments in java mode for <> (to insert html) but not inside code where these are less-than etc operators.

We can only "filter" context in specific mode (sp-local-pair "<" ">" :actions '(:wrap :insert) :filters '(sp-filter-in-code))

This will work so far as we don't add more distinctions than "in code/in string" (we might add in comment and users might supply their own enviroments).

So, there needs to be a list like :filters but with opposite semantics: only allow insertion if one of these predicates is true. What should it be called?

I'm also not sure about the name "filters" because I can see it meaning both things (inhibit list or allow list)... The name should be a noun in plural in both cases.

Edit: I've also decided to just break the backward compatibility and not re-define the old functions. The setup won't take more than 5 minutes for anyone (most people probably use only defaults or very close to defaults settings anyway), and it adds too much needless maintanance.

[ilohmar]

1. I am not sure one needs more than just a single "filters-like" keyword. Negation can be part of the predicate function, and it's easy to provide the inverse predicates, eg, sp-not-in-string-p, right?
2. As for the word itself, I think :when or :unless (depending on the negation) would be great (in analogy to the macros of the same name etc).
3. If you go for this, offering both keywords appears more intriguing again, now that I read it. I saw some issue with precedence rules here, but it was nonsense I think. Yeah, offering both sounds good, and should make the clause read almost like a sentence.

Personally, I do not see the need for the old functions as well. It is not like this is some 20+yrs configuration that people have to update :)

[Fuco1]

1. Well you can provide negative predicate, but that's not the same. Say I only want to trigger if the point is after a word (I don't know what use that'd be, but let's go with it). Now you would have to specify all the other possible scenarios where it should not happen, instead of just saying "when this is true, do stuff".

Which brings me to 2) that's pretty good! Not a noun, but adverb works too I guess. I just didn't want verbs as arguments, that's not good style.

3. preference rules will be simple, first test the "when" clause if it exist, then the "unless" clause if "when" doesn't exist. When you allow something it's (almost) always higher priority than giving a list of what you don't want.

So only one will be tested, but I don't think that's a problem. The combination of "when this unless this" can be expressed with a combined predicate in the when clause.

I'm also thinking about adding more macros, for example "sp-with-all-pairs". This would be useful if you wish to add a filter to all the pairs. This would be helpful if we decide to implement some of the "configure" options using filters. Which would be cleaner from the design perspective, but would require user to add the filter to all pairs. Which I don't like very much, flicking one toggle variable is much more user-friendly.

[ilohmar]

Ok I get the first point: Either in code that the package provides or in code the user writes one would have to list all other conditions then, that's not nice. So it is indeed best to offer both. Glad you like when and unless BTW.

Why not test both clauses? It's probably rarely necessary, but I think it would be a bad idea to allow both clauses to be present, and then one has no effect. I think the order does not really matter, does it? It is just inserting a pair if

(and (when-clause) (not (unless-clause)))

Since both clauses must not change context, point or whatever, the order of the test does not matter, or am I missing something here?

Adding macros like this seems a good idea. My take all the time was that the underlying system should be simple and transparent and available to the user---anything on top is icing, and macros make that /really/ easy.

[Fuco1]

Here's the default config using the new interface: https://gist.github.com/4675510 I've also split it in a separate file.

The 'nil' argument in local pairs... well. If you can supply key arguments without the key, I migth change it into (:close nil). To be honest I haven't tested it, but if it works I'll change it into that. Right now the 'nil' there means "I don't care, just pick this pair based on this opening delimiter and keep the closing one the same"

Yes, we can test both. I don't know why I thought it would cause problems :P

I'm getting nice progress, I think it's almost ready. I still don't have the tag stuff done, I'll probably change that later. Plus I need to update the action tests (right now the :wrap action is basically ignored). Other than that, it's basically ready!

[ilohmar]

Some minor remarks (just nitpicking):

1. nil for the global or existing closing delimiter makes a lot of sense, no matter the precise syntax. I think using nil or an explicit new string is more obvious than having a keyword :close at all...
2. Is there any reason you use colons in :wrap and :insert? I know about self-quoting, but personally I would find it less confusing if only the keywords have that form, and the values are plain symbols (usually one would quote the whole list anyway, like :actions '(wrap)
3. Do you keep sp-with-modes around for backwards compatibility? Because other than that, it seems to make things less straightforward than using the list of modes in sp-local-pair directly.
4. :actions defaulting to '(wrap insert) looks good, works the same as '(add) with a std global pair already present, right?
5. Have you considered naming the predicates according to elisp conventions, ie, sp-in-code-p etc?

Looks great so far! Really looking forward to the new version.

[Fuco1]

1. Yea, what I wanted isn't possible anyway, so this is the best we can do. You can either supply the closing pair each time, or just use nil to "use whatever there was before".

2. Ideally, I'd like it if I could simply do :actions (wrap insert) without any quoting. That's only possible if I add some macro wrappers around (I think). Which, thinking about it might be pretty cool. Or do you think it would be too confusing to new users (when to quote, when to not quote etc)?

Ok, let's make the actions just plain symbols. There wasn't any real reason for that. I'll still keep the "add/rem" special keywords as keywords because they have special semantics. Then I guess the context indicators will also change to just plain symbols.

So the actions will be: wrap, insert
The contexts: code, string

and the special operators: :add, :rem to facilitate the inheritance. Plus to possibly distinguish them from a function called "add" and "rem".

3. the sp-with-modes macro is new, old ones were called sp-with and sp-with-tag. The point is that if you want to call two functions with the same set of modes (and if you have a longer list) it is pretty ugly. Consider:

(sp-local-pair 
 '(emacs-lisp-mode inferior-emacs-lisp-mode lisp-interaction-mode scheme-mode common-lisp-mode) 
 "'" nil :actions nil)
(sp-local-pair
 '(emacs-lisp-mode inferior-emacs-lisp-mode lisp-interaction-mode scheme-mode common-lisp-mode)
 "`" nil :when '(sp-pred-in-string)))

Now imagine you customize 5 pairs in those modes, and then you start using clojure, so you have to edit 5 lists and make sure you don't forget something. Plus the repetition there is quite ugly :P

The sp-with-tag is pretty much obsolete now since all the customization is part of the sp-pair (sp-local-pair) function itself. I've renamed sp-with to sp-with-modes since I'd probably add more macros of that kind (like the mentioned sp-with-all-pairs)

4. Yep. I felt that if you just want to add a local pair, defining the actions is pretty much expected (why else would you add it in the first place). If it defaulted to :add, then it would inherit "nothing" from the (non-existant) global definition and so the pair wouldn't work at all, so that's not a good default in this case.

5. Well, I feel that this might cause some clashes with other packages/sp itself. I can very well see someone making a function called in-string-p (in fact, this function IS defined in thingatpt package :P). I guess prefixing them with sp- should fix the problem.

What could happen then is that someone might name their function same way as some internal sp function is named. I guess I'll just move all the internals to sp-- prefix to avoid this issue.

[Fuco1]

Ok, it's live. Now go find all the pesky bugs! :D

[ilohmar]

Very nice! I understand the reasoning behind sp-with-modes. Now, together with the updated default config, my personal setup is really simple to understand and just a few lines.

You did a really great job here. Thanks for all your work!

This is too small to warrant a separate issue: I am not sure how you handle which modes to add to sp-ignore-modes-list. Personally I add the various notmuch modes, a fairly nonstandard package I think, but I also put

help-mode info-mode occur-mode grep-mode compilation-mode

there.

[Fuco1]

I don't have any official policy for that. If the modes are not for text insertion, are built in or really common and some SP bindings might conflict, it can go on the list. Don't worry and just start the issue so we have some record on the changes.