Hello, my name is Stefan Monnier,
and I'm going to talk to you about--
well, I'm going to present a bit
of the life of a janitor.
So by and large, there's just
nothing to see here,
and that's probably
not super interesting,
but some of you might actually like to
see how I work, so I figured why not.
Usually what I do just doesn't make any
any significant difference,
and so I basically take existing code
that's working, and I try to change it
hopefully without breaking it too much
and make it slightly more...
you know, following some of the more
modern style, let's say,
and sometimes along the way,
it actually fixes some bugs.
More concretely, the kind of things
that I do is basically activate
lexical scoping--
that's really my main goal usually--
but also do things like convert
from `cl` to `cl-lib`,
sometimes I have to
fix some compilation dependencies,
I might convert from `defadvice` to `advice-add`,
and many of the things--
in terms of number of changes,
most of them are actually
changing `quote fun` to `hash quote fun`
because I prefer it,
but also it often helps me
have a better understanding
of which function is called where,
and so the warnings I get from it
sometimes help me. You look concretely...
it's not nothing really clear;
it's more in terms of helping me
have a mental image
of how the package works.
So let's take a look.
I'm going to start with
the package `heap`,
which I saw had a few weird things in it,
so I'm going to compile it.
That's basically the way the way I work,
right. I take a package.
I just pass it to the byte compiler.
I do that by just having
a clone of the whole
GNU ELPA repository,
and so that's why I built them.
I use the build rules
from the GNU ELPA repository.
These build rules enforce--
make sure that the files
are compiled in a clean environment
so you get fairly good warnings.
If you look at the warnings you see here,
there's a lot of things
which are completely irrelevant,
which are due to details
of the way I have my Emacs set up
and some of the local changes
I had in it so, you know,
there's no point
paying too much attention to it,
but here we have a first warning.
We see that this is using `cl`,
so we want to change this to `cl-lib`,
but that also means
that we may have a new dependency
on the `cl-lib` package,
so we have to go check
the start of the file
to see if it already declares
some dependency, and we see it doesn't,
not even on a on a recent-enough Emacs,
so we have to add--
sorry, that's not going very well...
oh...
okay, we're going to get there somewhere...
somehow...
oh, that still wasn't it, wow, okay--
and along the way...
Of course, since we converted to `cl-lib`,
we have to update the uses
so `defstruct` shouldn't be used anymore.
We may want to reindent this
to get something a bit cleaner.
We have here a missing quote...
hash, sorry.
We have `decf`, so `decf` is here,
and that needs to be replaced
with `cl-decf`.
Sometimes it's worth doing
a search-and-replace.
Here I see there's only two,
so it's not worth the trouble;
I just do it by hand, and that's it.
Well, that was easy.
So let's recompile, see what it says.
Ah, this is clean. Perfect!
Let's.. we can go see...
There is another one I had.
Was it `counsel`, I think. Yes.
So also I saw some funny things
going on here.
So I'm going to do
the same procedure as before:
I just compile the file
and look at the warnings.
Oh, we have many more here.
So let's see...
Okay, so we have missing quotes--
oh, hashes. They're not really missing;
it's just a personal preference.
Oh, here... here's an important one:
so as you know,
if you look at the top of the file,
you see that here
it says it's using lexical binding,
yet it's not fully using lexical binding,
because as we just saw,
there's a call to the `eval` function
with only one argument,
which means the second argument is nil,
which means that the expression read
by `read` here is going to be evaluated
using the old dialects,
which is only dynamic scoping.
So here I like to just change this
to use lexical scoping,
which in most cases
just doesn't make any difference.
It just makes me feel better.
So there's lots of those hashes
all over the place.
It's not strictly necessary, as you know,
but I'm just going to add them anyway.
Here we see
it's not going to warn me here
because it doesn't know
that `ivy-make-magic-action`
takes a function,
but it's a pretty good guess that it does.
And here's some more.
What else do we have?
Is that all we have here?
Well, looks like it. Oh, I see a few...
a few more here...
and one more.
And oh, this is more interesting.
So here we have a use of `defadvice`,
so if we go back
to the beginning of the file,
we see that it actually depends
on Emacs 24.5, so it actually has
the new advice system available
without having to add any dependency,
so there's really no good reason
to keep this.
So we just convert this
to an `advice-add`,
so it just says, you know,
this is the function that's advised.
This was a `before` advice.
The `before` advice, sometimes,
when we convert it to `advice-add`,
need to be converted to `around` advice.
This is when the function
looks or modifies the argument.
In this case, if I look at it,
I see it doesn't seem to be using
the arguments at all.
So I'm just going to keep it
as a `before` advice.
And we have to give it a name.
Well, we don't really have to,
but it's convenient to give it a name
to the new function.
So here, they actually had
given a name to the advice,
so we're going to keep it,
and indeed it's the only function.
This name is not used as a function,
so we can use it
as the name of the function.
I'm going to add a dash here
because I think this function
is really fundamentally
an internal function.
So here I just said I add the advice,
but I still need to actually
define the function.
So that's what I do here,
and we need here to list the arguments
that are going to be taken.
I don't know what these are,
but I know we're not using them,
so we'll just accept anything,
and that will do the trick.
It's a future-proof as well,
so that should work.
Oh, here we have another, so it's
basically the same story, I think.
It's a `before` advice as well.
It doesn't seem to be using
the argument at all,
and let's see if this name is not taken.
Yeah, good, so we can just do the same:
turn this into an `advice-add`...
`before`...
I just add a dash here.
And same thing--
a function that just takes...
because I don't know which arguments
these are so...
I think that should do the trick.
Actually, we see that this function
is very similar to the other one.
Let's look at the two side-by-side...
...it really is--
oh, it's not exactly identical...
it's, you know, we could try
to merge them into a single function,
but it's probably not worth the trouble
so we can keep it this way.
Okay, next warning: an `eval` again,
so I could just add `t` here,
but if you look at it a bit more,
you see that the code
we're going to evaluate
using either lexical scoping
or dynamic scoping
is actually just evaluating a symbol,
since we just call an `intern` here.
So instead of replacing this
by adding an argument,
I'm just going to call `symbol-value`
because that's exactly
what we need to do here, right.
I call this "strength reduction,"
and I'm using
a more primitive function instead,
which does just what we need,
and this one knows that it has to be
accessed by dynamic scoping, of course.
Here I have a `kmacro-ring`,
so here I have a function that uses--
`kmacro-ring` comes from
the `kmacro` package, obviously,
and we probably don't want to
`require` `kmacro` package
all over the place in `counsel` itself,
because `counsel` can be used
without `kmacro`.
So I think we're just going to add
a `defvar` to silence the warning.
And we have several more. So we have
`initial-counter-value`. (Sorry.)
We have `kmacro-counter`.
Do we have more?
Oh, yes, we do.
We have `kmacro-counter-value-start`
and `kmacro-counter-format-start`.
Okay.
I hope this is it.
`kmacro-ring`, `counter`, `ring`...
blah blah blah.
Here we have another one, `quote`.
Here we have another hash missing.
It's not missing...
but same thing here.
Okay, this is a function from `kmacro`.
We could declare it
just to silence the warning
although we don't actually...
normally, when we declare such things--
same thing with variables--
we should try to make sure that indeed
by the time the code is executed,
the function will be available,
and then very often is
because there's a `require`
sometimes inside a function,
and so we should put
the `declare` function
right after the `require`,
but I don't think it's the case here.
So I'm just going to to add this.
I know this comes from `kmacro`,
and I could actually check the arguments.
It's just taking an optional argument
so I'm going to put it there,
so we have it complete.
Okay, we can just recompile,
see what is left
from those warnings we've fixed,
and we may have new warnings, in any case,
because especially when we add the hashes,
it tends to give us more warnings.
So we have two more functions
which are not known.
You can just add them here...
`set-format "kmacro"`
and same thing for `set-counter`.
Okay, whatever.
This just takes a `format` argument,
and this one just takes an `arg` argument.
Okay, so let's see what this says now.
Hopefully, there's no warnings anymore.
We're done. Okay!
Okay, the last one we're going to see
is in `enwc`, I saw the other day...
I think I have it here...
here we go, yes...
so `enwc` is an interesting package here
because it has-- as you can see it has--
it's lexical binding,
but actually some of the files in it
do not use lexical binding,
so it has been partly converted
but not completely.
So here I'm going to
enable lexical binding.
I have also, I think, in `cm`...
yes...
so I enable it here,
and also, I think, `test`.
The test files are often
somewhat problematic
because very often they're not quite
as heavily tested themselves, actually,
or they only run
in very specific contexts,
and so they may have problems
with missing `requires` or using packages
which are not explicitly in the dependencies
and those kinds of things.
I think this is not the case here,
but we'll see.
`enwc`...
Yes, I want to save this one and that one.
Let's see what it says.
Okay, unused lexical variable `x`...
`x`...
Yes, so here we have an unused variable,
and indeed, it's not used.
It probably had to be named before
because it was...
with dynamic scoping,
the `dotimes` requires
the variable to be named, actually,
because it's used internally somehow,
but with lexical scoping,
that's not the case,
so we can just put an underscore.
I'm going to change this
because I really don't like
this three-part `dotimes`.
I prefer to have
the return value at the end.
It's sort of stashed hidden in the middle.
That's just a personal preference.
Okay, what else... we have a `widget`.
Okay, this argument here says that
it's not used, so if we look at...
We were here, right? Yes. Right here.
Indeed, `widget` is really not used.
(Sorry.)
Here's what I get for using
a somewhat vanilla configuration of Emacs,
compared to the one I use...
the personally tricked one.
Actually, I can...
so we can just mark this argument
as unused,
and we don't want to remove the argument
probably, or maybe we could;
we could see where the function is used,
and here we see that it's passed
to a higher-order function,
basically, so it's going to be...
We can't really change
the calling convention
so we have to mark the argument
as being just an unused argument,
but we're going to still receive it.
And here it says same thing:
that `widget` is not used in this function.
Let's take a look at the function.
Indeed it seems it's not used,
and so we're just going to mark it
as unused.
This is the part of the conversion
to lexical scoping
that's somewhat tricky sometimes
because we don't really know
whether this variable should be using
lexical scoping or dynamic scoping.
The fact that it's not used
is a hint that there's probably
something going on,
so either it's not used
because it should be using
dynamic scoping--
it is going to be used
by some other code somewhere else--
or it's really not used
because it's just not used, right,
and so we need to distinguish the two,
and for that, I basically use
my own judgment.
This is based typically on the fact that
this is just a very short name,
and most local identifiers use short names,
whereas item values used for dynamic scoping
typically have a package prefix
or something like this.
So the fact that it's a short name
gives me a good idea.
Here in this case,
I actually look at the code,
and we see that there's nothing in here
that may actually refer
to this variable `widget`,
so I think it's safe,
but in the general case,
we may look here and be surprised,
or, you know, you may call out
the functions which may themselves end up
referring to this variable.
So sometimes we need to investigate a
little more.
We are most of the time not completely sure
whether the result is correct or not,
of course, so the other thing
you may want to check
is also uses of things
like `eval` or `symbol-value`.
So it's often a good idea to search,
and you do a search of `eval`,
and you see here it's using `eval`.
Hmmm... Okay, so what does this `eval` do?
It's evaluating expressions
that appear in `args` here
so you can see where those args come from,
and we see here, these are expressions
that don't do anything very special.
It's just using `make-supplicant-choice`,
and `make-supplicant-choice` itself
just doesn't refer to `widget`, for example,
so you know we should be safe,
but while I'm here...
okay, well, then we can do that later.
Well, that's actually the next warning,
exactly. So here we see that this is
using the dynamically-scoped dialect,
so we convert it to lexical-scoped.
Of course, this may introduce errors,
but we hope it doesn't.
And actually, it was a good change here,
because if you see again,
this actually evals expressions
that appear here in `args`,
and so these are expressions
that are passed here.
So this expression here used to be
evaluated with dynamic scoping,
even though it appears to be normal code
within this file, which says
it's using lexical scoping,
and so there are some remnants
of dynamic scoping all over the place
in Emacs still, because we have
those calls of `eval` with a nil argument.
Here we have `cons`...
that needs to be `hash quoted`.
Oh, and we have a reference
to this variable `enwc-edit-id'.
So this is clearly
a dynamic-scoped variable.
We can either add a `defvar`
to silence the warning,
or maybe we can `require` the package.
The file that defines it...
So let's see where it's defined.
Here it's defined in `enwc.el`,
so I'm going to try just to add
the dependency.
I'm going to `require` here. This is risky.
We'll see when we compile a file later,
we may get a circular dependency
because of it.
If that's the case, we're going to
have to remove this `require`
and instead put `defvar`s.
Sometimes it's worth actually
looking further at the various files
to see how to redefine the dependencies
to break those circular dependencies,
but it's often not really
worth the trouble.
Oh, no, that's not what--
I'm not going to the right place...
Here I was. So here `edit-map`.
Well, we can probably...
it may disappear or...
oh, I see.
Okay, so this `edit-map` actually is
defined in this very file.
It's just that it's defined later.
So all we need to do
is to move this definition
to before its first use,
since otherwise it's going to be taken
as lexically-scoped, which we don't want.
And while I'm here, I see this `copy-keymap`.
I don't like `copy-keymap`,
so I'm going to change this
to a normal keymap,
and then I'm just going to use
`set-keymap-parent` instead of `copy-keymap`
to get basically the same result,
but without having copied anything.
And this one will disappear...
this one as well-- or should hopefully,
thanks to the `require`.
Here we have a `hash` missing,
and we have some functions
which are unknown,
so let's see...
Where is this function defined?
Nowhere. Huh, wonderful, okay.
So we'll just leave it like it is,
and that's going to be
for the author of the package to fix.
How about this one?
Oh, okay, so it's defined in `enwc.el`
so presumably,
this is going to disappear as well.
One more...
Okay, so this one
is just like the previous one.
We're going to leave it at that.
And this is it! Huh, wonderful.
So let's recompile.
Oh, we have a warning for `fin`.
This variable seems not to be used
anywhere in the file, so we're just
going to remove it.
I leave it there just in case
someone needs later on
to look for a `fin` variable
to see where it used to be.
Again, you know, maybe it's actually used...
yeah, dynamic scoping somehow,
but given the short name,
I presume this is not the case.
Here, oh, that's the code removed
that had a hash missing.
That's the one that's not defined.
This one is not defined,
and this is it.
Let's make a last recompilation
to see if we missed yet something else.
Nope, and that's it, okay.
Well, here we go; we're done.
Okay so this was it.
You've seen, I think,
pretty much examples of all of those,
and I hope you enjoyed it.
Lessons to take home:
use the byte compiler.
You can also use `flymake-mode` instead.
I recommend enabling it as much as you can,
and head the warnings.
Follow the warnings. Try to fix them.
If you can fix all of the warnings,
it's always much better,
because then the new warnings
really show up.
And once you've done it, it's really
kind of-- because there's always
new things coming up.
And I think this is it.
I hope you liked it, and thank you
for attending this presentation. Bye.
[captions by Hannah Miller]