On 5 July 2013 13:30, Dmitry Gutov wrote: > On 05.07.2013 13:56, Bozhidar Batsov wrote: > >> The patch looks good with one minor problem - you've included `autoload` >> twice (the second time it's in Module's methods). >> > > Thanks for the catch. > > > While on the subject of small improvements here's a few more ideas: >> >> * replace cl with cl-lib (that should be done eventually I guess) >> > > Yeah, I guess. > > > * update the front-matter comment since it's pretty out-of-date >> > > What exactly do you propose to change? If the file is installed manually, > and if it's not autoloaded, the user has to add some auto-mode-alist > entries. > > I thought the manual installation instructions were leftovers from the old ruby-mode. I was under the impression that since ruby-mode started using SMIE it's not a good idea to distribute it separately, since it might not behave appropriately on older Emacsen. Perhaps I'm wrong. > > * run a whitespace cleanup on the code :-) >> > > Untabify, you mean? It's the core's policy not to touch tabs vs. spaces > until you meaningfully modify nearby code. That's unfortunate. Files with tabs look like Christmas trees for most whitespace-mode users. > > > * highlight yard and rdoc special syntax in comments - like Emacs Lisp >> does >> > > Have you looked at https://github.com/pd/yard-**mode.el/? > > The only problem with it I can see is, it sets > eldoc-documentation-function, so it can't be used outside of comments. I > guess we'll eventually need eldoc-documentation-functions hook, like we > have for completions. Yes, I have - but I really feel that this is something that should be handled by ruby-mode itself. After all around 90% of all ruby hackers have to deal with yard and rdoc. It's better to have core functionality built-in. > > > * make use of `font-lock-negation-char-face` for ! >> > > Uh, okay. Is it different from the default face, in any themes? If more modes were actually using it more themes would have customised it. I can assure you that zenburn and solarized will support it :-) > > > * maybe make of use of new `font-log-regexp-grouping-**construct-face` >> as well >> > > I think highlighting backslash sequences is more important, and the two > approaches are somewhat incompatible. See: > > http://debbugs.gnu.org/cgi/**bugreport.cgi?bug=14481 > https://github.com/dgutov/**highlight-escape-sequences I see. > > > P.S. Please try to keep emacs-devel in Cc. > Sorry about that. I'm trying to use a new mail client these days and I'm not totally used to it.