[Emacs-diffs] master 9ed3685a77: Lots of refactorings and a few minor improvements.

[Emacs-diffs] master 9ed3685a77: Lots of refactorings and a few minor improvements.

[Tino Calancha]

>Lots of refactorings and a few minor improvements.
>
>User visible improvements and changes:
>* Improve and debug `rst-forward-section` and `rst-backward-section`.
>* Auto-enumeration may be used with all styles for list insertion.
>* Improve and debug `rst-toc-insert`.
>* Adapt change in Emacs to use customization group `text` instead of `wp`.
>* Bind `n` and `p` in `rst-toc-mode`.
>* `z` in `toc-mode` returns to the previous window configuration.
>* Require Emacs version >= 24.1.
>
>Lots of refactorings including:
>* Silence byte compiler.
>* Use lexical binding.
>* Use `cl-lib`.
>* Add tests and raise test coverage.
This is not a standard Emacs commit message: i need to go throughout
the whole diffs to know exactly what entities have being modified.

Tino

Re: [Emacs-diffs] master 9ed3685a77: Lots of refactorings and a few minor improvements.

[Paul Eggert]

Tino Calancha wrote:

> This is not a standard Emacs commit message: i need to go throughout
> the whole diffs to know exactly what entities have being modified.

Yes. Please see the section "Commit messages" in the file CONTRIBUTING for the 
commit-message format that is preferred for Emacs changes.

Re: [Emacs-diffs] master 9ed3685a77: Lots of refactorings and a few minor improvements.

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]

With these many changes, I believe it is worth to make another commit with
a proper change log as explained here:
http://git.savannah.gnu.org/cgit/emacs.git/tree/CONTRIBUTE#n44

One should be able to understand which files (and functions) are touched by
that commit just by reading the commit log.

On Tue, Jan 3, 2017 at 10:05 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

> Tino Calancha wrote:
>
> > This is not a standard Emacs commit message: i need to go throughout
> > the whole diffs to know exactly what entities have being modified.
>
> Yes. Please see the section "Commit messages" in the file CONTRIBUTING for
> the
> commit-message format that is preferred for Emacs changes.
>
> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1319 bytes --]

Re: [Emacs-diffs] master 9ed3685a77: Lots of refactorings and a few minor improvements.

[Paul Eggert]

Stefan Merten wrote:

> I got the impression, that lately few people care about these rules

I'm afraid that impression is incorrect. Many developers care (hence this thread 
:-).

> See for example:
>
> http://git.savannah.gnu.org/cgit/emacs.git/commit/lisp/textmodes/rst.el?id=92e5b41c7c5898820356fc66456804a45bbe7852
> http://git.savannah.gnu.org/cgit/emacs.git/commit/lisp/textmodes/rst.el?id=c61ee94959ba96b2a327df0684593f7e569e30be

Those examples merely rename locals or adjust comments, and typically this 
doesn't need to be mentioned in ChangeLogs. Your changes did more than that.

> I hope the commit message below is what you require.

Much better, thanks. My main problem with it is that "Refactor by removing old 
functions, introducing new functions and change existing functions" is too 
vague. Please write for an audience that includes people who have code that 
calls (say) rst-comment-region and want to know how to adjust the code after 
your changes to the API.

At this point, by the way, the commit is done. We're talking about what you 
should do next time. (You can change the ChangeLog.3 file later if you like, but 
that's not urgent.)

Refactoring in rst.el (was: Re: [Emacs-diffs] master 9ed3685a77: Lots of refactorings and a few minor improvements.)

[Stefan Merten]

Hi Paul!

3 days ago Paul Eggert wrote:
> Stefan Merten wrote:
>> I got the impression, that lately few people care about these rules
> 
> I'm afraid that impression is incorrect. Many developers care (hence
> this thread :-).

Thanks for reassuring. As you can see in the log I once did care but
this time thought it is no longer worth the hassle.

>> I hope the commit message below is what you require.
> 
> Much better, thanks. My main problem with it is that "Refactor by
> removing old functions, introducing new functions and change existing
> functions" is too vague. Please write for an audience that includes
> people who have code that calls (say) rst-comment-region and want to
> know how to adjust the code after your changes to the API.

Which raises the question what the API of `rst.el` is. So far I was
convinced that nobody will use any function from `rst.el` - so I'd
think that nobody is impacted by refactoring even if I change
signatures of functions. Of course I may be wrong.

Apart from that I'd think the interactive functions are the *real*
interface of `rst.el` which is interesting. Refactoring, however, by
definition does not impact this behavior. Debugging and those minor
changes in behavior which I did during refactoring I did document,
however. Where key bindings are impacted I defined aliases for renamed
functions so older key bindings should continue to work.

In other languages you have this public / private concept. I remember
that in Emacs you started using a double dash in identifiers to mark
them private. For example

	rst-comment-region

would be part of the public interface, while

	rst--comment-region

would be an internal function not meant for public use.

Is that correct? Are the examples valid?

If so, I'd probably like to mark internal functions this way. That
would be a good idea IMHO.


						Grüße

						Stefan

Re: Refactoring in rst.el

[Paul Eggert]

Stefan Merten wrote:

> Which raises the question what the API of `rst.el` is.

When in doubt, document the change in the ChangeLog.

> I remember
> that in Emacs you started using a double dash in identifiers to mark
> them private.

Yes. The part in front of the double-dash is the module name.

> If so, I'd probably like to mark internal functions this way. That
> would be a good idea IMHO.

Yes, that's a reasonable change. It'd be a change to the API, and should be 
documented in the ChangeLog and in any other documentation to the package.

After that happens, it's a good idea to document even changes to the internal 
API, for benefit of those maintaining the code in question.

Re: Refactoring in rst.el

[Stefan Merten]

Hi Paul!

Last week (7 days ago) Paul Eggert wrote:
> Stefan Merten wrote:
>> I remember
>> that in Emacs you started using a double dash in identifiers to mark
>> them private.
[...]
>> If so, I'd probably like to mark internal functions this way. That
>> would be a good idea IMHO.
[...]
> After that happens, it's a good idea to document even changes to the
> internal API, for benefit of those maintaining the code in question.

Do you know of anybody who is maintaining the code (beyond me)?


						Grüße

						Stefan