files.el: Patch to make project-settings optional/customizable

files.el: Patch to make project-settings optional/customizable

[T.V. Raman]

Introduces a defcustom for project-settings-file -- rather than
hard-coding the name,
and if set to nil, arranges for function
project-find-settings-file to do nothing.

Index: files.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/files.el,v
retrieving revision 1.1007
diff -u -r1.1007 files.el
--- files.el	30 Oct 2008 04:32:22 -0000	1.1007
+++ files.el	13 Nov 2008 04:53:05 -0000
@@ -3201,6 +3201,12 @@
     (if elt
 	(setcdr elt list)
       (push (cons class list) project-class-alist))))
+(defcustom project-settings-file ".dir-settings.el"
+  "Settings file for per-project settings. Set this to nil if you
dont want  to load project-specific settings."
+  :type '(choice
+       (const  :tag "Ignore" nil)
+       (file :tag "File"))
+:group 'files)

 (defun project-find-settings-file (file)
   "Find the settings file for FILE.
@@ -3209,8 +3215,9 @@
 If the file is in a registered project, a cons from
 `project-directory-alist' is returned.
 Otherwise this returns nil."
+  (when project-settings-file
   (setq file (expand-file-name file))
-  (let* ((settings (locate-dominating-file file ".dir-settings.el"))
+  (let* ((settings (locate-dominating-file file project-settings-file))
          (pda nil))
     ;; `locate-dominating-file' may have abbreviated the name.
     (if settings (setq settings (expand-file-name ".dir-settings.el"
settings)))
@@ -3223,7 +3230,7 @@
         (if (> (length (file-name-directory settings))
                (length (car pda)))
             settings pda)
-      (or settings pda))))
+      (or settings pda)))))

 (defun project-define-from-project-file (settings-file)
   "Load a settings file and register a new project class and instance.

-- 
Best Regards,
--raman


Email:  raman@users.sf.net
WWW:    http://emacspeak.sf.net/raman/
AIM:    emacspeak       GTalk: tv.raman.tv@gmail.com
PGP:    http://emacspeak.sf.net/raman/raman-almaden.asc
Google: tv+raman
IRC:    irc://irc.freenode.net/#emacs

Re: files.el: Patch to make project-settings optional/customizable

[Chong Yidong]

"T.V. Raman" <tv.raman.tv@gmail.com> writes:

> Introduces a defcustom for project-settings-file -- rather than
> hard-coding the name, and if set to nil, arranges for function
> project-find-settings-file to do nothing.

Looks reasonable.  I've checked it in, thanks.

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > "T.V. Raman" <tv.raman.tv@gmail.com> writes:
  > 
  > > Introduces a defcustom for project-settings-file -- rather than
  > > hard-coding the name, and if set to nil, arranges for function
  > > project-find-settings-file to do nothing.
  > 
  > Looks reasonable.  I've checked it in, thanks.

Shouldn't the variable be called dir-settings-file ?
The default name is .dir.settings.el and this is documented in a
"Directory Variables" section in the manual after all...

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Dan Nicolaescu <dann@ics.uci.edu> writes:

  > Chong Yidong <cyd@stupidchicken.com> writes:
  > 
  >   > "T.V. Raman" <tv.raman.tv@gmail.com> writes:
  >   > 
  >   > > Introduces a defcustom for project-settings-file -- rather than
  >   > > hard-coding the name, and if set to nil, arranges for function
  >   > > project-find-settings-file to do nothing.
  >   > 
  >   > Looks reasonable.  I've checked it in, thanks.
  > 
  > Shouldn't the variable be called dir-settings-file ?
  > The default name is .dir.settings.el and this is documented in a
  > "Directory Variables" section in the manual after all...

On second thought: is the goal of this change just to disable the search
for .dir-settings.el ?

If yes, then maybe another variable should be used for that.

If no, what is the advantage of having this name customizable?  
What happens if different projects using different names?
It seems that it would just bring in complications...

Re: files.el: Patch to make project-settings optional/customizable

[Tom Tromey]

>>>>> "Dan" == Dan Nicolaescu <dann@ics.uci.edu> writes:

Dan> If no, what is the advantage of having this name customizable?  
Dan> What happens if different projects using different names?
Dan> It seems that it would just bring in complications...

Yes, this makes the feature less useful, by making it not work
automatically across projects.

Before making any change here, I would ask that people go read the
several long threads about the naming of this file first.  I don't
recall if this particular idea was discussed, but I do know that the
current code was the outcome of a lot of back-and-forth.

Tom

Re: files.el: Patch to make project-settings optional/customizable

[Chong Yidong]

Tom Tromey <tromey@redhat.com> writes:

> Dan> If no, what is the advantage of having this name customizable?  
> Dan> What happens if different projects using different names?
> Dan> It seems that it would just bring in complications...
>
> Yes, this makes the feature less useful, by making it not work
> automatically across projects.

I think it's generally good policy to avoid hardcoding file names.  The
default is unchanged, so the feature still works automatically across
projects unless you change it.  People who care enough about changing
the default presumably know what they are doing (presumably, they want
to have a different file name for some reasons of their own, and this
change lets them do that without redefining the functions).

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > Tom Tromey <tromey@redhat.com> writes:
  > 
  > > Dan> If no, what is the advantage of having this name customizable?  
  > > Dan> What happens if different projects using different names?
  > > Dan> It seems that it would just bring in complications...
  > >
  > > Yes, this makes the feature less useful, by making it not work
  > > automatically across projects.
  > 
  > I think it's generally good policy to avoid hardcoding file names.  

But general policy should not be applied without thinking about the
particulars of each case.
As Tom said, there were some long discussions about this, and I think
that the opposite conclusion was reached _for this particular case_.
Just undoing that decision without reviewing the previous discussions on
this does not seem like a good idea.

  > The default is unchanged, so the feature still works automatically
  > across projects unless you change it.  People who care enough about
  > changing the default presumably know what they are doing

Then shouldn't we first wait for such people to say that they want such
a feature and show us why?
Configurations is good only if it has some benefits, what is the benefit
in this case?
Adding more knobs to turn just increases the complexity of emacs...

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Thu, Nov 13, 2008 at 18:28, Dan Nicolaescu <dann@ics.uci.edu> wrote:

> Then shouldn't we first wait for such people to say that they want such
> a feature and show us why?

As a first datapoint, ".dir-settings.el" is ugly as hell to my eyes. I
wouldn't dream of having Emacs-specific files in a source tree and not
naming them ".emacs-WHATEVER".

> Adding more knobs to turn just increases the complexity of emacs...

You could use the same argument against savehist-file,
bookmark-default-file, recentf-save-file, etc. The fact is, we have
lots of that kind of variables because configuration file's names is
something people cares about a lot (witness a recent help-emacs thread
were the OP wanted to change not only .emacs.d/, but the name of
.emacs / init.el).

  Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

"Juanma Barranquero" <lekktu@gmail.com> writes:

  > On Thu, Nov 13, 2008 at 18:28, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > 
  > > Then shouldn't we first wait for such people to say that they want such
  > > a feature and show us why?
  > 
  > As a first datapoint, ".dir-settings.el" is ugly as hell to my eyes. I
  > wouldn't dream of having Emacs-specific files in a source tree and not
  > naming them ".emacs-WHATEVER".

As Tom said, we've already had this discussion, is there a good reason
to restart it now?

Re: files.el: Patch to make project-settings optional/customizable

[Chong Yidong]

Dan Nicolaescu <dann@ics.uci.edu> writes:

>   > I think it's generally good policy to avoid hardcoding file names.  
>
> But general policy should not be applied without thinking about the
> particulars of each case.  As Tom said, there were some long
> discussions about this, and I think that the opposite conclusion was
> reached _for this particular case_.  Just undoing that decision
> without reviewing the previous discussions on this does not seem like
> a good idea.

I can't seem to find the discussion.  Could you point me to it?

(Even if we decide to hardcode the name, which I'm not convinced about,
there needs to be a comment in the code explaining why.)

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > Dan Nicolaescu <dann@ics.uci.edu> writes:
  > 
  > >   > I think it's generally good policy to avoid hardcoding file names.  
  > >
  > > But general policy should not be applied without thinking about the
  > > particulars of each case.  As Tom said, there were some long
  > > discussions about this, and I think that the opposite conclusion was
  > > reached _for this particular case_.  Just undoing that decision
  > > without reviewing the previous discussions on this does not seem like
  > > a good idea.
  > 
  > I can't seem to find the discussion.  Could you point me to it?

I don't have anything handy, I'd guess that searching for
.dir-settings.el and Tom's name should be good enough.

  > (Even if we decide to hardcode the name, which I'm not convinced about,
  > there needs to be a comment in the code explaining why.)

Say one works on 
emacs  -- that has (not now, but we should add it) .dir-settings.el that sets c-set-style to "gnu"
linux  -- that might have .dir-settings.el that sets c-set-style to "linux"
.emacs that sets project-settings-file to "my-cool-name-for-project-settings.el"
my-own-project  -- that has my-cool-name-for-project-settings.el that
                   sets c-set-style to "k&r"

after that when working in my-own-project the C style is set correctly,
and it is silently ignored for emacs and linux.  Sort of defeats the
purpose of having this.

Re: files.el: Patch to make project-settings optional/customizable

[Tom Tromey]

>>>>> "Juanma" == Juanma Barranquero <lekktu@gmail.com> writes:

Juanma> As a first datapoint, ".dir-settings.el" is ugly as hell to my eyes. I
Juanma> wouldn't dream of having Emacs-specific files in a source tree and not
Juanma> naming them ".emacs-WHATEVER".

RMS specifically asked that "emacs" not appear in the name.
He thought it was redundant given the ".el" extension, which, as I
recall, he asked for.

http://lists.gnu.org/archive/html/emacs-devel/2007-09/msg00667.html

Tom

Re: files.el: Patch to make project-settings optional/customizable

[Tom Tromey]

>>>>> "Chong" == Chong Yidong <cyd@stupidchicken.com> writes:

Chong> I can't seem to find the discussion.  Could you point me to it?

There were several.

Here is the root of one:

    http://lists.gnu.org/archive/html/emacs-devel/2007-09/msg00328.html

Here is another:

    http://lists.gnu.org/archive/html/emacs-devel/2007-09/msg00863.html

This may be the original thread:

    http://lists.gnu.org/archive/html/emacs-devel/2007-07/msg00168.html

Tom

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Thu, Nov 13, 2008 at 18:59, Dan Nicolaescu <dann@ics.uci.edu> wrote:

> As Tom said, we've already had this discussion, is there a good reason
> to restart it now?

Yes. Because you've asked

"Then shouldn't we first wait for such people to say that they want such
a feature and show us why?"

and I've answered. You shouldn't ask if you don't want answers.

             Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

> RMS specifically asked that "emacs" not appear in the name.
> He thought it was redundant given the ".el" extension, which, as I
> recall, he asked for.

Even if Dan just implied otherwise, I'm not restarting the discussion,
nor am I proposing changing the name.

But adding a defcustom and letting people customize it to their
hearts' content, that's a good idea, as it is for other packages'
files which we *do* allow to customize.

             Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Tom Tromey]

>>>>> "Chong" == Chong Yidong <cyd@stupidchicken.com> writes:

Chong> People who care enough about changing the default presumably
Chong> know what they are doing (presumably, they want to have a
Chong> different file name for some reasons of their own, and this
Chong> change lets them do that without redefining the functions).

The issue is that some project will check in files with the "wrong"
name.

Then we are back in the same situation.  If you want emacs to "just
work" across projects, you must write special code in your .emacs to
recognize different settings in different directories.

IOW, then we have the same problem we started out to solve.

Tom

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Thu, Nov 13, 2008 at 20:23, Tom Tromey <tromey@redhat.com> wrote:

> The issue is that some project will check in files with the "wrong"
> name.
>
> Then we are back in the same situation.  If you want emacs to "just
> work" across projects, you must write special code in your .emacs to
> recognize different settings in different directories.
>
> IOW, then we have the same problem we started out to solve.

Surely 99% of project never ever change the name. Let's assume that
the ones which do know what they are doing.

   Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Dan Nicolaescu <dann@ics.uci.edu> writes:

  > Chong Yidong <cyd@stupidchicken.com> writes:
  > 
  >   > Dan Nicolaescu <dann@ics.uci.edu> writes:
  >   > 
  >   > >   > I think it's generally good policy to avoid hardcoding file names.  
  >   > >
  >   > > But general policy should not be applied without thinking about the
  >   > > particulars of each case.  As Tom said, there were some long
  >   > > discussions about this, and I think that the opposite conclusion was
  >   > > reached _for this particular case_.  Just undoing that decision
  >   > > without reviewing the previous discussions on this does not seem like
  >   > > a good idea.
  >   > 
  >   > I can't seem to find the discussion.  Could you point me to it?
  > 
  > I don't have anything handy, I'd guess that searching for
  > .dir-settings.el and Tom's name should be good enough.
  > 
  >   > (Even if we decide to hardcode the name, which I'm not convinced about,
  >   > there needs to be a comment in the code explaining why.)

Any news about this?

  > Say one works on 
  > emacs  -- that has (not now, but we should add it) .dir-settings.el that sets c-set-style to "gnu"
  > linux  -- that might have .dir-settings.el that sets c-set-style to "linux"
  > .emacs that sets project-settings-file to "my-cool-name-for-project-settings.el"
  > my-own-project  -- that has my-cool-name-for-project-settings.el that
  >                    sets c-set-style to "k&r"
  > 
  > after that when working in my-own-project the C style is set correctly,
  > and it is silently ignored for emacs and linux.  Sort of defeats the
  > purpose of having this.

Re: files.el: Patch to make project-settings optional/customizable

[Juri Linkov]

>   >   > >   > I think it's generally good policy to avoid hardcoding file names.
>   >   > >
>   >   > > But general policy should not be applied without thinking about the
>   >   > > particulars of each case.  As Tom said, there were some long
>   >   > > discussions about this, and I think that the opposite conclusion was
>   >   > > reached _for this particular case_.  Just undoing that decision
>   >   > > without reviewing the previous discussions on this does not seem like
>   >   > > a good idea.
>   >   >
>   >   > I can't seem to find the discussion.  Could you point me to it?
>   >
>   > I don't have anything handy, I'd guess that searching for
>   > .dir-settings.el and Tom's name should be good enough.
>   >
>   >   > (Even if we decide to hardcode the name, which I'm not convinced about,
>   >   > there needs to be a comment in the code explaining why.)
>
> Any news about this?

I agree with Juanma that the name .dir-settings.el is "ugly as hell".
Perhaps this is the reason why users want to customize it.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Juri Linkov <juri@jurta.org> writes:

  > >   >   > >   > I think it's generally good policy to avoid hardcoding file names.
  > >   >   > >
  > >   >   > > But general policy should not be applied without thinking about the
  > >   >   > > particulars of each case.  As Tom said, there were some long
  > >   >   > > discussions about this, and I think that the opposite conclusion was
  > >   >   > > reached _for this particular case_.  Just undoing that decision
  > >   >   > > without reviewing the previous discussions on this does not seem like
  > >   >   > > a good idea.
  > >   >   >
  > >   >   > I can't seem to find the discussion.  Could you point me to it?
  > >   >
  > >   > I don't have anything handy, I'd guess that searching for
  > >   > .dir-settings.el and Tom's name should be good enough.
  > >   >
  > >   >   > (Even if we decide to hardcode the name, which I'm not convinced about,
  > >   >   > there needs to be a comment in the code explaining why.)
  > >
  > > Any news about this?
  > 
  > I agree with Juanma that the name .dir-settings.el is "ugly as hell".
  > Perhaps this is the reason why users want to customize it.

You did read the part that showed that as things stand now, if the file
name is changed, the feature completely breaks?

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Tue, Nov 18, 2008 at 23:15, Dan Nicolaescu <dann@ics.uci.edu> wrote:

> You did read the part that showed that as things stand now, if the file
> name is changed, the feature completely breaks?

I think you forgot the last part, "...if you mix and match projects
not knowing what you're doing". Didn't you?

  Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Stefan Monnier]

> I agree with Juanma that the name .dir-settings.el is "ugly as hell".
> Perhaps this is the reason why users want to customize it.

The admin subdir "CVS" has a name that's very troublesome, yet CVS
offers no way to change it, and it's a good thing.

If you want to change .dir-settings.el, the say so and re-open the
discussion about the choice.  But making it customizable is wrong.


        Stefan

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

"Juanma Barranquero" <lekktu@gmail.com> writes:

  > On Tue, Nov 18, 2008 at 23:15, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > 
  > > You did read the part that showed that as things stand now, if the file
  > > name is changed, the feature completely breaks?
  > 
  > I think you forgot the last part, "...if you mix and match projects
  > not knowing what you're doing". Didn't you?

And this is currently documented ... where?  And such fragile behavior
is preferable because ... ?

Re: files.el: Patch to make project-settings optional/customizable

[Juri Linkov]

>> I agree with Juanma that the name .dir-settings.el is "ugly as hell".
>> Perhaps this is the reason why users want to customize it.
>
> The admin subdir "CVS" has a name that's very troublesome, yet CVS
> offers no way to change it, and it's a good thing.
>
> If you want to change .dir-settings.el, the say so and re-open the
> discussion about the choice.  But making it customizable is wrong.

I propose to do two things:

1. change `project-settings-file' from defcustom to defvar,
   thus making it non-customizable, but still leaving a way to
   modify it for knowledgeable people when necessary;

2. find a better file name that it both short and easy to connect
   to the Emacs feature that uses it.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: files.el: Patch to make project-settings optional/customizable

[Chong Yidong]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> If you want to change .dir-settings.el, the say so and re-open the
> discussion about the choice.  But making it customizable is wrong.

How about making it a non-customizable variable (with a comment saying
why it's non-customizable)?

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Tue, Nov 18, 2008 at 23:59, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> If you want to change .dir-settings.el, the say so and re-open the
> discussion about the choice.  But making it customizable is wrong.

Re-opening the discussion is wrong; it was already discussed and some
consensus reached (

Making it user-settable (perhaps, as Chong says, as a variable and not
a defcustom) is best than trying to second-guess the users.

  Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
  > 
  > > If you want to change .dir-settings.el, the say so and re-open the
  > > discussion about the choice.  But making it customizable is wrong.
  > 
  > How about making it a non-customizable variable (with a comment saying
  > why it's non-customizable)?

What is the advantage of that?
If someone is so offended by the .dir-settings.el name, they can:
-avoid using it
-defadvice project-find-settings-file (or reimplement it altogether)
there's not reason to add complexity to emacs code and documentation
because of that.

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Wed, Nov 19, 2008 at 00:18, Dan Nicolaescu <dann@ics.uci.edu> wrote:

> And this is currently documented ... where?

Nowhere it is documented that changing it could cause problems, as it
is nowhere documented that the name is cast in stone because [at least
part of] the developers "know better". Adding a note to the docstring
is not difficult, though.

> And such fragile behavior
> is preferable because ... ?

It's not more "fragile" than allowing local variables in files, which
can be a security liability. And yet we do allow it, taking
appropriate measures to minimize the risks.

  Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Wed, Nov 19, 2008 at 00:41, Dan Nicolaescu <dann@ics.uci.edu> wrote:

> What is the advantage of that?
> If someone is so offended by the .dir-settings.el name, they can:

And why are you so offended by a defvar? It doesn't even have to be
documented, other than a docstring:

project-settings-file is a variable defined in `files.el'.
Its value is ".dir-settings.el"

Documentation:
Settings file for per-project settings.
Internal use. Don't change it unless you understand the consequences.

  Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

"Juanma Barranquero" <lekktu@gmail.com> writes:

  > On Wed, Nov 19, 2008 at 00:41, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > 
  > > What is the advantage of that?
  > > If someone is so offended by the .dir-settings.el name, they can:
  > 
  > And why are you so offended by a defvar? 

Because it's not a variable, it has to be a constant.  (And a string
literal as it was until a few days ago is even better.)
In a few years someone will see the defvar and will want to turn it into
a defcustom and we'll have the same discussion again.

  > It doesn't even have to be
  > documented, other than a docstring:

Each extra variable adds a  bit of complexity, and there's no shortage
of that.

Re: files.el: Patch to make project-settings optional/customizable

[Chong Yidong]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> Because it's not a variable, it has to be a constant.  (And a string
> literal as it was until a few days ago is even better.)
> In a few years someone will see the defvar and will want to turn it into
> a defcustom and we'll have the same discussion again.

Just add a comment to the code, pointing to this discussion.

> Each extra variable adds a bit of complexity, and there's no shortage
> of that.

The complexity is negligible.  This is a matter of good programming
practice.

Re: files.el: Patch to make project-settings optional/customizable

[Stefan Monnier]

>> If you want to change .dir-settings.el, the say so and re-open the
>> discussion about the choice.  But making it customizable is wrong.

> Re-opening the discussion is wrong; it was already discussed and some
> consensus reached (

> Making it user-settable (perhaps, as Chong says, as a variable and not
> a defcustom) is best than trying to second-guess the users.

Do you advocate making "Local Variables:" in hack-local-variables
customizable as well?  If you think one deserves customization, then I'd
argue that the other deserves it just as well.

As far as I can tell, the only reason people want to customize it is
because they find the default choice ugly.  I.e. for purely aesthetic
reasons.  This is a pretty poor justification for introducing complexity
and incompatibilities between projects.

Until someone gives me a good argument for why it should be customizable
(as opposed to having just a different value), we should revert the
patch that made it a defcustom.

As for its name: we're not wedded to it, AFAIK.  It was the result of
a pretty long thread, so it's not going to be easy to find something
else that's more or less consensual.  But for starters, if people could
argue about why it's "ugly", maybe we can tweak it.


        Stefan

Re: files.el: Patch to make project-settings optional/customizable

[Stephen J. Turnbull]

Juanma Barranquero writes:

 > Making it user-settable

It *is* user-settable, at worst it requires rebuilding Emacs and we
all do that several times a day.<wink>

 > (perhaps, as Chong says, as a variable and not a defcustom) is best
 > than trying to second-guess the users.

Sometimes compromise is bad, and it is in this case.  Making this a
variable is admittedly broken by design.  Let those who know enough to
work around the breakage prove it by adding the feature in their
user or site init files if they want to.

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Wed, Nov 19, 2008 at 02:48, Chong Yidong <cyd@stupidchicken.com> wrote:

> Just add a comment to the code, pointing to this discussion.

[...]

> The complexity is negligible.  This is a matter of good programming
> practice.

Agreed.

  Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Wed, Nov 19, 2008 at 03:01, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Do you advocate making "Local Variables:" in hack-local-variables
> customizable as well?  If you think one deserves customization, then I'd
> argue that the other deserves it just as well.

I'd argue that, too, but the uses are quite different. Most "Local
Variables:" sections I will encounter are not of my doing; if I change
that string, I'll have incompatibilities all around.

But that is not true with ".dir-settings.el". I'm not involved in many
shared projects, nor likely to be. Bets are that all (or almost all)
.dir-settings.el I'll ever find will be my own, in my own projects. It
makes perfect sense for me to change that name to something "better"
(for subjective values of better). If my assumption is wrong and I
find that I'm really running into incompatibilities, I can change it
back. But the incompatibility is theoretic; the wish to use a better
name is real.

> As far as I can tell, the only reason people want to customize it is
> because they find the default choice ugly.  I.e. for purely aesthetic
> reasons.  This is a pretty poor justification for introducing complexity
> and incompatibilities between projects.

The complexity of a defvar is negligible (as I've said, I don't think
it merits being documented in the manual). As for aesthetic reasons,
that's why we allow changing almost every filename or path used by
most Emacs packages.

And, regarding incompatibility: why do you assume the first thing
people will do is change the value? It is really hard for the user to
grasp a comment saying "DO NOT change unless you're aware of the
consequences."?

> As for its name: we're not wedded to it, AFAIK.  It was the result of
> a pretty long thread, so it's not going to be easy to find something
> else that's more or less consensual.  But for starters, if people could
> argue about why it's "ugly", maybe we can tweak it.

I'm no big believer in restarting already settled discussions for
"aesthetic reasons". That's why I strongly believe it is better to let
those who want to change it to do so: because there is no good value
that will look nice to everyone. No amount of discussion is going to
change that.

  Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Richard M. Stallman]

    Because it's not a variable, it has to be a constant.  (And a string
    literal as it was until a few days ago is even better.)
    In a few years someone will see the defvar and will want to turn it into
    a defcustom and we'll have the same discussion again.

defconst seems appropriate in this case.

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > >> If you want to change .dir-settings.el, the say so and re-open the
  > >> discussion about the choice.  But making it customizable is wrong.
  > 
  > > Re-opening the discussion is wrong; it was already discussed and some
  > > consensus reached (
  > 
  > > Making it user-settable (perhaps, as Chong says, as a variable and not
  > > a defcustom) is best than trying to second-guess the users.
  > 
  > Do you advocate making "Local Variables:" in hack-local-variables
  > customizable as well?  If you think one deserves customization, then I'd
  > argue that the other deserves it just as well.
  >
  > As far as I can tell, the only reason people want to customize it is
  > because they find the default choice ugly.  I.e. for purely aesthetic
  > reasons.  This is a pretty poor justification for introducing complexity
  > and incompatibilities between projects.
  > 
  > Until someone gives me a good argument for why it should be customizable
  > (as opposed to having just a different value), we should revert the
  > patch that made it a defcustom.

Done.

Re: files.el: Patch to make project-settings optional/customizable

[Juri Linkov]

> As for its name: we're not wedded to it, AFAIK.  It was the result of
> a pretty long thread, so it's not going to be easy to find something
> else that's more or less consensual.  But for starters, if people could
> argue about why it's "ugly", maybe we can tweak it.

Looking at the current documentation and function names we can see
a wide diversity of used terms:

  directory local variables
  dir-settings
  variable settings
  project-bindings
  project settings
  directory-project
  project classes (that have nothing to do with OOP!)

All this is very confusing.

In http://thread.gmane.org/gmane.emacs.devel/78366/focus=78378
Richard said that the term "project" is misleading and asked to
rename "project" in the documentation and the function names.
But for unknown reasons that was not really done for the function names.

Also in http://thread.gmane.org/gmane.emacs.devel/78366/focus=78404
Stefan asked to rename "project" to "directory variables" or
"directory-local variables" that was eventually renamed in the manual but
not in code.  And even Tom tended to think that "project" is a misnomer ;)
http://thread.gmane.org/gmane.emacs.devel/77852/focus=78054

So let's get rid of "project" in the function names and use the common
prefix dir-local-variables by analogy with file-local-variables.
This also suggest renaming "bindings" and "settings" to "variables".

Since the only term the user have to remember will be "directory-local
variables" this suggests using a file name derived from it.  Since
the file name should be short a good candidate is ".dir-locals.el".

It is similar to the file name used by the good package
http://www.loveshack.ukfsn.org/emacs/dir-locals.el that is now
obsoleted after the same functionality is implemented now in files.el.
But dir-locals.el uses a different file name, so ".dir-locals.el" has no
conflict with this package and can be considered as inheriting a good
file name from it.

I've also checked in long threads
http://thread.gmane.org/gmane.emacs.devel/74257
http://thread.gmane.org/gmane.emacs.devel/77852
http://thread.gmane.org/gmane.emacs.devel/78366
http://thread.gmane.org/gmane.emacs.devel/97396
that the file name ".dir-locals.el" doesn't contradict
the reached consensus, because

1. it is still a dotfile with a leading . in its name,
2. the file name ends with `.el',
3. there is no `emacs' in the name.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: files.el: Patch to make project-settings optional/customizable

[Tom Tromey]

>>>>> "Juri" == Juri Linkov <juri@jurta.org> writes:

Juri> In http://thread.gmane.org/gmane.emacs.devel/78366/focus=78378
Juri> Richard said that the term "project" is misleading and asked to
Juri> rename "project" in the documentation and the function names.
Juri> But for unknown reasons that was not really done for the function names.

Sorry about that.  I probably just forgot that RMS asked for the
function names to be changed as well.

Tom

Re: files.el: Patch to make project-settings optional/customizable

[Stefan Monnier]

> So let's get rid of "project" in the function names and use the common
> prefix dir-local-variables by analogy with file-local-variables.

Sounds OK.

> Since the only term the user have to remember will be "directory-local
> variables" this suggests using a file name derived from it.  Since
> the file name should be short a good candidate is ".dir-locals.el".

Fine by me,


        Stefan

Re: files.el: Patch to make project-settings optional/customizable

[Kevin Rodgers]

Juri Linkov wrote:
> So let's get rid of "project" in the function names and use the common
> prefix dir-local-variables by analogy with file-local-variables.
> This also suggest renaming "bindings" and "settings" to "variables".

Rational.  Thank you!

-- 
Kevin Rodgers
Denver, Colorado, USA

Re: files.el: Patch to make project-settings optional/customizable

[Juri Linkov]

>> So let's get rid of "project" in the function names and use the common
>> prefix dir-local-variables by analogy with file-local-variables.
>
> Sounds OK.
>
>> Since the only term the user have to remember will be "directory-local
>> variables" this suggests using a file name derived from it.  Since
>> the file name should be short a good candidate is ".dir-locals.el".
>
> Fine by me,

Done.

Since seeing constant file names repeated in several places hurts my eyes,
I also created defconst for the file name, as Richard suggested - with the
docstring saying why it is constant.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Juri Linkov <juri@jurta.org> writes:

  > >> So let's get rid of "project" in the function names and use the common
  > >> prefix dir-local-variables by analogy with file-local-variables.
  > >
  > > Sounds OK.
  > >
  > >> Since the only term the user have to remember will be "directory-local
  > >> variables" this suggests using a file name derived from it.  Since
  > >> the file name should be short a good candidate is ".dir-locals.el".
  > >
  > > Fine by me,
  > 
  > Done.

Thanks for doing this!

  > Since seeing constant file names repeated in several places hurts my eyes,
  > I also created defconst for the file name, as Richard suggested - with the
  > docstring saying why it is constant.

Since dir-locals-file is used in exactly one function, it would be
better to just let bind it there.  
And defconsts are not exactly constants in elisp, so if someone does a
(setq dir-locals-file BLAH), the problem that started this whole
discussion still happens.  More, we don't have defconsts for 
"Local Variables"  or "-*-" which serve a similar purpose but at file
level.

Re: files.el: Patch to make project-settings optional/customizable

[Juri Linkov]

> Since dir-locals-file is used in exactly one function, it would be
> better to just let bind it there.
> And defconsts are not exactly constants in elisp, so if someone does a
> (setq dir-locals-file BLAH), the problem that started this whole
> discussion still happens.  More, we don't have defconsts for
> "Local Variables"  or "-*-" which serve a similar purpose but at file
> level.

I don't care how constant constants are.  Function definitions are not
constant either, so everyone can redefine the function with own file name
and have the same problem ;-)  IOW, defconst says the user should not complain
when the user changes its value, so I see no problems with defconst.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Juri Linkov <juri@jurta.org> writes:

  > > Since dir-locals-file is used in exactly one function, it would be
  > > better to just let bind it there.
  > > And defconsts are not exactly constants in elisp, so if someone does a
  > > (setq dir-locals-file BLAH), the problem that started this whole
  > > discussion still happens.  More, we don't have defconsts for
  > > "Local Variables"  or "-*-" which serve a similar purpose but at file
  > > level.
  > 
  > I don't care how constant constants are.  Function definitions are not
  > constant either, so everyone can redefine the function with own file name
  > and have the same problem ;-)  IOW, defconst says the user should not complain
  > when the user changes its value, so I see no problems with defconst.

How about the more important fact that this particular constant has
exactly one user (and just two instances in that single user), the
common practice for that is to just let bind it in the function that
uses it.
Sorry, but this feels like trying to sneak in something that has been
explicitly rejected...

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Sat, Nov 22, 2008 at 21:20, Dan Nicolaescu <dann@ics.uci.edu> wrote:

> Sorry, but this feels like trying to sneak in something that has been
> explicitly rejected...

Not by Richard, who asked for the defconst.

             Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Miles Bader]

Dan Nicolaescu <dann@ics.uci.edu> writes:
> Sorry, but this feels like trying to sneak in something that has been
> explicitly rejected...

I agree it does feel like that, but still, making it a defconst seems
like the tasteful thing to do...

After all, it's hardly unthinkable that some other frob or another will
want to notice the .dir-settings.el file.

-Miles

-- 
Electricity, n. The cause of all natural phenomena not known to be caused by
something else.

Re: files.el: Patch to make project-settings optional/customizable

[Stefan Monnier]

> I agree it does feel like that, but still, making it a defconst seems
> like the tasteful thing to do...

We could make it read-only, like we do for t, nil, :toto, and
enable-multibyte-characters.


        Stefan

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 06:27, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> We could make it read-only, like we do for t, nil, :toto, and
> enable-multibyte-characters.

Are you also going to forbid redefining or defadvising
`dir-locals-find-file'? ;-)

             Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Stefan Monnier]

>> We could make it read-only, like we do for t, nil, :toto, and
>> enable-multibyte-characters.
> Are you also going to forbid redefining or defadvising
> `dir-locals-find-file'? ;-)

No.


        Stefan

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > > I agree it does feel like that, but still, making it a defconst seems
  > > like the tasteful thing to do...
  > 
  > We could make it read-only, like we do for t, nil, :toto, and
  > enable-multibyte-characters.

Or just get rid of the defconst.  Let's look at the code in question:

(defconst dir-locals-file ".dir-locals.el"
  "File that contains directory-local variables.
It has to be constant to enforce uniform values
across different environments and users.")

(defun dir-locals-find-file (file)
  "Find the directory-local variables FILE.
This searches upward in the directory tree.
If a local variables file is found, the file name is returned.
If the file is already registered, a cons from
`dir-locals-directory-alist' is returned.
Otherwise this returns nil."
  (setq file (expand-file-name file))
  (let ((locals-file (locate-dominating-file file dir-locals-file))
	(dir-elt nil))
    ;; `locate-dominating-file' may have abbreviated the name.
    (when locals-file
      (setq locals-file (expand-file-name dir-locals-file locals-file)))
    (dolist (elt dir-locals-directory-alist)
      (when (and (eq t (compare-strings file nil (length (car elt))
					(car elt) nil nil))
		 (> (length (car elt)) (length (car dir-elt))))
	(setq dir-elt elt)))
    (if (and locals-file dir-elt)
	(if (> (length (file-name-directory locals-file))
	       (length (car dir-elt)))
	    locals-file
	  dir-elt)
      (or locals-file dir-elt))))

So there are 2 uses .dir-locals.el 3 lines apart in the same 24 lines function.
The defconst adds 4 lines.  It's hard to keep a straight face and claim
that the defconst is a good idea.   
(and the 2 uses could be reduced to a single one if we make
locate-dominating-file return an expanded file name -- which seems to be
a good idea anyway).

It's kind of painful that this discussion is still going on, it should
have never occurred in the first place...

Re: files.el: Patch to make project-settings optional/customizable

[Juri Linkov]

> So there are 2 uses .dir-locals.el 3 lines apart in the same 24 lines function.
> The defconst adds 4 lines.  It's hard to keep a straight face and claim
> that the defconst is a good idea.
> (and the 2 uses could be reduced to a single one if we make
> locate-dominating-file return an expanded file name -- which seems to be
> a good idea anyway).
>
> It's kind of painful that this discussion is still going on, it should
> have never occurred in the first place...

I can't believe we have this kind of discussion :(

All packages with similar functionality have defcustom for the file name.

dir-locals.el:

(defcustom dir-locals-file-name ".emacs-locals"
  "File name used by Dir-Locals mode to specify local variables.
This should specify local variables in the normal way.  When Dir-Locals
minor mode is active, these will be inherited by files found in a
directory tree containing such a file at its root.
This may also be a function of no arguments which returns the name to
use, allowing arbitrary per-directory customization of the
per-directory customization file on the basis of `default-directory'."
  :group 'dir-locals
  :type '(choice file function))

dirvars.el:

(defcustom dirvars-file-name ".emacs-dirvars"
  "File base name that is loaded by dirvars."
  :type 'string)

dir-locals.el was created in 2005 and dirvars.el in 2002.
For several years they provided customization without problems.

Our discussion reached the need to make this file name constant.
I'm ok with this conclusion.  So I created defconst because it
explicitly says that the file name should not be modified by the user,
unlike if it was hidden in the function dir-locals-find-file where users
will be the temptation to redefine the whole function without understanding
that the file name have to be constant.

PS: Reading the docstring of dir-locals-file-name in dir-locals.el
seems to indicate that users trying to switch from dir-locals.el
to similar functionality implemented now in files.el will be annoyed
by its limitations but I'll leave this for another discussion where
we will argue with the users of these packages.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Juri Linkov <juri@jurta.org> writes:

  > > So there are 2 uses .dir-locals.el 3 lines apart in the same 24 lines function.
  > > The defconst adds 4 lines.  It's hard to keep a straight face and claim
  > > that the defconst is a good idea.
  > > (and the 2 uses could be reduced to a single one if we make
  > > locate-dominating-file return an expanded file name -- which seems to be
  > > a good idea anyway).
  > >
  > > It's kind of painful that this discussion is still going on, it should
  > > have never occurred in the first place...
  > 
  > I can't believe we have this kind of discussion :(
  > 
  > All packages with similar functionality have defcustom for the file name.
  > dir-locals.el was created in 2005 and dirvars.el in 2002.
  > For several years they provided customization without problems.

This has been discussed to death, and someone said it very well upthread:
"it's broken by design".

  > Our discussion reached the need to make this file name constant.

How was it not a constant before the change?

  > I'm ok with this conclusion.  So I created defconst because it
  > explicitly says that the file name should not be modified by the user,

What does that accomplish that the previous code or adding a comment to
the code did not?  
Why is this better encapsulation?

  > unlike if it was hidden in the function dir-locals-find-file where users
  > will be the temptation to redefine the whole function without understanding
  > that the file name have to be constant.

Why is the danger of redefining a function greater than simply using setq? 

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 13:10, Dan Nicolaescu <dann@ics.uci.edu> wrote:

> Why is the danger of redefining a function greater than simply using setq?

The danger here is that when the original function is improved, the
redefined one is not unless the user notices the changes.

             Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

"Juanma Barranquero" <lekktu@gmail.com> writes:

  > On Sun, Nov 23, 2008 at 13:10, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > 
  > > Why is the danger of redefining a function greater than simply using setq?
  > 
  > The danger here is that when the original function is improved, the
  > redefined one is not unless the user notices the changes.

And the reason emacs should care about that is... ? 

Re: files.el: Patch to make project-settings optional/customizable

[Juanma Barranquero]

On Sun, Nov 23, 2008 at 18:16, Dan Nicolaescu <dann@ics.uci.edu> wrote:

> And the reason emacs should care about that is... ?

Because you're not stopping the user from changing the
.dir-settings.el name, just making all the issue more brittle. I
thought the reason you insisted on having a hardcoded .dir-settings.el
was to protect users.

  Juanma

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

"Juanma Barranquero" <lekktu@gmail.com> writes:

  > On Sun, Nov 23, 2008 at 18:16, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > 
  > > And the reason emacs should care about that is... ?
  > 
  > Because you're not stopping the user from changing the
  > .dir-settings.el name, just making all the issue more brittle. I
  > thought the reason you insisted on having a hardcoded .dir-settings.el
  > was to protect users.

Brittle is to allow the user to break something just by using a normal
way of changing settings "setq".  We have gone over this too many times
already... 

Re: files.el: Patch to make project-settings optional/customizable

[Stephen J. Turnbull]

Dan Nicolaescu writes:

 > Brittle is to allow the user to break something just by using a normal
 > way of changing settings "setq".  We have gone over this too many times
 > already... 

Agreed, making a defconst is not childproof (which seems to be the
goal of several senior developers these days).  But I thought Emacs
Lisp was a language used by adults?  You expect adults to read the
documentation and take responsibility for what they do.  I really
don't see a problem with using a defconst here.

BTW, "brittle" doesn't mean that it breaks when the user uses normal
features of Lisp to intentionally do something inadvisable.  "Brittle"
means it breaks *accidentally* in the normal process of programming.

Making this a defconst is the opposite of brittle: it is just good
programming practice, as it allows the byte compiler to check for
typos etc. and warn about attempts to set or bind the variable.  You
can't do that with a string.  (Well, you *could*, but then it would be
a symbol, wouldn't it?)

Re: files.el: Patch to make project-settings optional/customizable

[Dan Nicolaescu]

Juri Linkov <juri@jurta.org> writes:

  > > So there are 2 uses .dir-locals.el 3 lines apart in the same 24 lines function.
  > > The defconst adds 4 lines.  It's hard to keep a straight face and claim
  > > that the defconst is a good idea.
  > > (and the 2 uses could be reduced to a single one if we make
  > > locate-dominating-file return an expanded file name -- which seems to be
  > > a good idea anyway).
  > >
  > > It's kind of painful that this discussion is still going on, it should
  > > have never occurred in the first place...
  > 
  > I can't believe we have this kind of discussion :(

Yeah, it could have been avoided by not adding the defconst, which is
quite obvious by now that was only added to circumvent the fact that the
settings file name should not be changeable by the user.  And that is
not just a whim, it's an issue of robustness and quality of
implementation.

More, adding the defconst does not fix any bug, so it shouldn't have
been done at this point.

Even more, it does cost something: the pure size increases by 28 bytes,
and the data size of emacs by 8K (might have hit a page boundary
condition). 

As shown by the actual code, it is more lines of code, so it is not
simplifying anything.  But it does make it easy to break the feature.

So please revert that change.

Re: files.el: Patch to make project-settings optional/customizable

[Chong Yidong]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> the pure size increases by 28 bytes, and the data size of emacs by 8K
> (might have hit a page boundary condition).
>
> As shown by the actual code, it is more lines of code, so it is not
> simplifying anything.  But it does make it easy to break the feature.
>
> So please revert that change.

Please drop this.