Thank you Basil for your prompt response! Your patches LGTM. Let's wait few more days if some people want to make further comments. Otherwise, I will push them in your name into the master branch. On Wed, 9 May 2018, Basil L. Contovounesios wrote: >> I) >>> +(declare-function comint-output-filter "comint" (process string)) >>> + >> What is the purpose of this? AFICT no warning is shown when compiling >> the file. > > On my end, removing the function declaration and invoking 'make' results > in the following output: > > ... > make[2]: Entering directory '/home/blc/.local/src/emacs/lisp' > ELC ../lisp/simple.elc > Reloading stale loaddefs.el > Loading /home/blc/.local/src/emacs/lisp/loaddefs.el (source)... > > In end of data: > simple.el:9030:1:Warning: the function ‘comint-output-filter’ is not known to > be defined. > ... > > Note that this warning is only emitted when comint-output-filter is > #'-quoted. This is in line with the usual behaviour of the > byte-compiler that I am accustomed to, i.e. I don't see anything out of > the ordinary here. Oh, I see. I overlooked the fact that we changed `quote' with `function'. With: make bootstrap there is no warning. I am OK with clearing the warning in all cases. >> * We require `shell.el' inside `shell-coomand'. >> * `shell.el' requires `comint.el'. > > Yes, I understand that using comint-output-filter at this point in the > program is kosher, but the byte-compiler evidently does not. > >> Is the purpose to serve as documentation? In that case I don't think we >> need it (the prefix 'comint-' already makes obvious where this function >> belongs to). > > No, the only intention is to pacify the byte-compiler. > >> II) >> It's better to keep consistent with the indentation of the function you >> are modifying: here, `shell-command' is indenting with TAB. >> >> Tip: >> You can see the tabs searching them with: >> C-s C-q C-I >> or you can persistenly highlight them with: >> M-s h r C-I RET RET > > Thanks for the tip. I personally prefer to use [global-]whitespace-mode > with a whitespace-style setting which includes (face tab-mark). > I additionally avoid accidentally committing tabs by configuring the git > option core.whitespace to include tab-in-indent. > >> For instance, here you are changing: >> 1) ' ---> #' >> ;; and >> 2) \t\t\s\s 000> \s\s\s\s...\s (18 white spaces) >> >> Please, do not change 2). > > I have no strong feeling on this; I was merely going along with the > (emacs-lisp-mode . ((indent-tabs-mode . nil))) setting in the project's > toplevel dir-locals-file, as well what I had inferred to be accepted > policy (as Noam mentions in a separate email) from following Emacs > development for the last couple of years. > > If it weren't for the above and the fact that most everything I have > come across in the Emacs tree has Frankindentation (including the target > function shell-command), I would be more inclined to remain consistent > with the surrounding source. > > Let me know if it's still a problem and I'll gladly resend the patches > with indent-tabs-mode enabled. It's OK, Frankenstein is a glorious novel after all; I recommend anyone to revisit such classical masterpiece :-)