-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] [WIP]: Proposed file reorg #11
Conversation
- Use XCB definitions from `/usr/share/xcb` by default. - Move distributed files into a lisp subdirectory. - Make a `test` directory. - Rename `el_client.el` to `xelb_gen.el`.
Looks good to me. Does this require changes in the ELPA registry? If yes we should synchronize with Stefan Monnier. EDIT: Changes are needed. Check out the elpa-packages file in the elpa repository. |
Thinking about this more - we don't need to restructure the directory if all we want is to ignore tests. We can add them to elpaignore. Personally I maintain a flat directory hierarchy for my packages, so I am not sure regarding the advantages of a restructuring. |
|
I see. For me a nested hierarchy is slightly more complicated, we have to adjust elpa-packages etc. What I want to say - there are also complications and not only advantages. For me, introducing a hierarchy doesn't seem urgent or that important, given that the root directory is not very cluttered. Furthermore I like to avoid superfluous changes - it is better to somewhat maintain current conventions as long as we don't hit limitations, but maybe we already do? |
I think it depends on how we are going to proceed with the tests. Will there be a single xelb-tests.el (my preference) or tens of *-test.el files? If we will only have xelb-gen.el and xelb-tests.el, introducing a hierarchy doesn't seem justified. |
I don't think we'll need more than one, may be two here. My main concern is installing from source. What if I put just the tests in a subdirectory? That would keep them out of the load-path. |
If we have only a single or two test files, it seems artificial to create a subdirectory. Having the test file on the load path won't hurt if you use something like package-vc. This makes it even easier to load it and run tests (which you want if you mix development and installation). For Compat we have the exact same flat layout and it hasn't led to problems so far. If for some reason, you want to have a cleaner installation, then my suggestion is to not use package-vc. The problem is that package-vc conflates development and installation. This creates clutter both ways. Your development directory contains compiled files and your installation directory contains possibly unwanted development files. If I recall correctly, there has been a discussion on emacs-devel where people wanted to install from a separate git repository. This is what I am doing. I have a simple command which let's me select from my directories in ~/projects/elisp/, make a package out of it and install it. |
For my part I'm OK with what you both decide. For me the biggest issue is differentiating the
I think that's an improvement, as a first-timer won't have to first download xcb-proto to the right place. If possible, include a comment pointing to the xcb-proto repository as a hint for someone wanting to generate a newer version. E.g.:
I guess that's OK; users don't need it.
I didn't see tests in #4. |
Tests: #4 (comment) (ish).
Including random files can cause byte compiler warnings. Although, we can (and should) disable byte compilation for the test/build files. On the other hand, I'm kind of confused why you're so keen on keeping a flat layout, most complex lisp projects have multiple levels of directories: magit (lisp & test), ement (test), pdf-tools (lisp & test), forge (lisp & test), plz (test), evil (scripts dir). Yes, package-vc isn't perfect, but it works and it's very convenient for testing. And yes, a few test/build files in my load-path won't cause major issues, but having subdirectories isn't going to cause any major issues either. |
@Stebalien Let me first say that I am sorry that we are over-discussing this Regarding changes in general or this change in particular, I am keen on keeping A flat hierarchy is the simplest layout. It works like this without any The examples you mentioned are of course great packages and prime examples, but Regarding package-vc - the paradigm of package-vc is opinionated, with its I think the problems being discussed here can all be solved and should not
Ultimately this issue boils downs to opinion and preference. Both alternatives |
If this were a massive refactor, I'd agree. But that's why I proposed just moving the I've given my motivation for this change: I have a real and existing issue with the current layout. Sure, it may be minor, but it's an actual issue. Unfortunately "package-vc is bad" doesn't make my point any less valid: package-vc is part of Emacs core, works 99% of the time, and there's no good reason not to support it. I'm asking for clarification because (a) the current situation is a problem for me and (b) I can't argue with "I don't like it". |
I also gave good reasons I think why I prefer a flat layout and that a flat layout introduces fewer problems. In the end it is preference, but there are also technical arguments which we have to weigh. Can you please clarify which pressing issue you have with the existing layout or why you want to move files out of the way? I think I do not understand this sufficiently well. I argue that there should not be an issue with a flat layout. All .el files should compile cleanly and having them on the load path is an advantage for development and maintenance. If files do not compile cleanly, the solution is not to move them out of the way, but to make sure that they compile cleanly. This is a maintenance issue which I consider important. I also do not want to clutter the repository with random files or directories which in the end only contain a single file. I do not argue that package-vc is bad, but the design is opinionated, limited and ignores lessons from other systems. Now we rediscover potential problems which had already been solved. Furthermore a flat layout works well with package-vc and does not introduce compilation problems (as demonstrated by Compat), and may be even preferable since you have access to everything, which is advantageous for development. |
I guess I missed that? I'm really just trying to find a compromise here, but I can't without knowing your full position. All I've seem so far is that you prefer a flat layout, but that doesn't help me find a compromise. If I knew why you had this preference, I'd be able to make progress.
I'd hardly call it pressing, but it's annoying:
These are, of course, minor issues. But they're real issues. I get not wanting to move all the lisp files to a separate directory, but moving just the tests and elisp scripts to subdirectories is pretty trivial, solves my issue, and seems to be how most projects handle this. |
Thanks, I understand. We could make sure that el_client.el (better named xelb-gen.el) and xelb-tests.el compile cleanly and can be loaded without negative effects. Having them on the load path should be even less of a problem then. Even right now, random files on the load path will not get loaded automatically. However we can do better, improving the script and creating a proper Elisp feature out of it. Would this be acceptable to you? Overall I see three proposals:
|
That's likely to complicate executing it. We'd have to run it with
Neither of those projects have a How about we start with a minimal version of 2:
|
Execution will not necessarily become more complicated. We can detect script execution by inspecting
I prefer not to do this. Let me recapitulate. The problem is that we have the Elisp file el_script, which is not a proper feature or Elisp library. If it gets loaded, it may break Emacs. Having the file on the load path is not a problem in itself, but still a little bit dangerous, since the file could get accidentally loaded more easily. Now we have two ways to handle this.
Both 1 and 2 solve the problem we have. I prefer Elisp libraries without undesired side effects since this eases maintenance. I consider it a good practice in general. One can evaluate or load the file without the fear of breaking something. Having the ability to load a library makes it possible to use it directly from within Emacs which eases testing by hand. One may also evaluate during editing. For example for completion, one necessarily has to evaluate the file to make the symbols available. Not relevant for el_script, but if there are macros in a file, one has to evaluate the file to get proper macro indentation in the following code (To be precise, as a workaround one can only evaluate the macros one by one). Additionally I prefer to not introduce sub directories if we don't have to, in the hope that we don't add further clutter in the future, but this is a minor concern.
Yes, such examples are not unusual. But Elisp scripts, which are not proper libraries, are not necessarily a good practice. I don't know if the aforementioned scripts belong to this class. Proper Elisp libraries have advantages as I pointed out. el_client.el is almost there. It is all functions, even all of them live in the xelb- namespace. The only problem is the side effect, which we can easily remove. Therefore - would approach 2 of turning el_client into a proper library be acceptable or not? Note that we have a similar situation in Exwm. There we have exwm-config.el which you probably also don't want to use, but fortunately it does not have side effects such that loading it will not break Emacs. |
You may feel that way, but I don't see that sentiment in the rest of the community. We even got a new How about making it not look like a library?
That'll cause Emacs to exclude it from the load-path.
Not cleanly. I guess one option is to split it into two:
Unfortunately, the
You're not going to get good results unless its run in a clean environment. We could rewrite it to avoid these side-effects, but that would be adding a bunch of complexity for no good reason.
|
To be precise - I mean to check
This is not a feeling. I have written preference a few time, but please don't disregard the technical arguments I gave. I gave arguments why a library is advantageous over a script. Yes, one can also write quick scripts but this is not better. A library has advantages when maintaining it, which I listed above. Which advantages do you see if we make it a script? If There exist many maintenance tools in the Emacs world which are not scripts but proper libraries, e.g., elpa-admin.el, package-lint.el and so on. Still, I appreciate the introduction of -x. It is good to extend our toolbox. The introduction is also understandable, given that many people write shell scripts and one could also wish to write Elisp scripts instead. Yet, the better approach is to write libraries. |
This is certainly an argument. I have to look at el_client in more detail to evaluate what this would entail. Hopefully the change would not increase complexity, that's my hypothesis. In order to see, I would have to apply the changes.
As a compromise and to resolve the situation here, I think I can agree to rename el_client.el to xelb-gen and do as you propose, if you are okay if I may try to turn xelb-gen.el into a library later on. It could very well be that this increases complexity or turns out to be unfeasible, but it shouldn't hurt to try? To be clear, this is not urgent, and I probably won't do this soon. I rather want to work on actual features and fix some bugs. |
Until I Import the library from another script, then the library thinks it's running as a script and everything breaks.
There's merit in factoring out complex logic from a script to:
But that doesn't mean scripts are "bad practice". Furthermore, scripts serve a different purpose:
Yes, you could spend a lot of time trying to turn the script into a library, but then:
The cost (initial time, increased complexity, cost of maintenance, etc.) of making it a proper side-effect free library vastly exceeds the benefit of doing so.
Those aren't scripts, they're reusable libraries.
I'm happy with this compromise. |
No, I assure you that this would not happen if the check is done carefully enough. One would have to to check the command line for both --script and xelb-gen.el.
I don't want to go deeper into the discussion here, but I am looking at this with my "Emacs glasses" on. For me Emacs functions and Emacs libraries can be used to replace the usual scripts which are common if you use a Unix-style workflow centered around a shell with custom utility scripts. A script becomes a library or a function which I can load into Emacs and run it via a key binding, eval or via M-x. This may sound a little absurd, but we are talking about an Emacs desktop here, which one can build with EXWM and some other packages. The script el_client we are discussing here is almost a normal Elisp library. Also some of the tools we commonly refer to as scripts gradually evolve with time. At first a tool starts as a simple sequential shell script, then it may get replaced by some more maintainable Python script and in the next step the functionality will be extracted in a library with classes and all the common abstractions. At that point the script is reduced to a simple launcher which instantiates some library classes and calls library methods.
Of course. This could happen, but it remains to be seen if the complications would be significant. My hypothesis is that this would not happen, given that el_client.el is an "almost Elisp library". But I already said, I don't consider this high priority. Like you, I am convinced that our time should better be spent elsewhere.
Well, the boundaries are fluid. I think in the case of elpa-admin.el I actually disagree. If you look at the corresponding GNUMakefile, elpa-admin.el is used in exactly the same way as we are using el_client.el - as a script invoked via a Makefile. Yet, elpa-admin.el is structured like a regular Elisp library, which brings the aforementioned advantages during development (completion, jumping to definition, macro expansion and so on).
Thanks, me too! It is definitely the easiest solution to just rename el_client.el to xelb-gen and be done with it for now. :) |
That's way too hacky. If we absolutely need a library and a script, we should just write a small script that imports the library. With respect to libraries v. scipts, it comes down to not over complicating things for the sake of "reusability". If you try to make a script into a reusable library before you need it to be a reusable library, I guarantee you you'll:
I.e., premature abstraction. |
I think we just disagree on these things. I tried to explain my thoughts. I gave examples (python scripts, elpa-admin.el). There is no exact right or wrong on this matter. But you are shrugging these arguments of as premature abstraction. Why is this necessary? These generic terms "premature abstraction" or "premature optimization" are not useful. They are a general guideline and that's it. Writing something like xelb-gen.el and elpa-admin.el as libraries is not something I call "premature abstraction". The code is not intended to be reusable, but writing it in the form of a library has advantages, I've mentioned multiple times now (no danger when evaluating, completion, jump to definition, macro expansion, ...). We can ask Stefan Monnier why he wrote elpa-admin.el the way he did. I don't think the goal was to make the code reusable, but rather more maintainable and safer. Maybe there is also a misunderstanding. When I say library, I don't mean an elaborate construct. A file without side effects, with a provide statement and a few defuns already makes up an Elisp library. In the end we agree that time should not be wasted. |
Because this script is not the same as those scripts. This code was not designed to run as a library (it says so right in the code) and, by not trying to be a library, it can save a fair amount of complexity. The code:
None of that would work as a library. It needs to be executed in a clean Emacs environment, and works by evaling code in that environment. Look, if it were easy to rewrite it as a library, I'd say:
You're absolutely right that Emacs's builtin lisp editing features work best for libraries. But it isn't a library, it currently gets added to the load path, and making it a library would be more work than it's worth. |
Thanks, this makes sense. The way the environment is mutated saves complexity for the script in its current form and it makes it difficult to get rid of the side effects. I agree with that. One could probably store the state in a separate obarray or use another data structure. I really cannot tell how much complexity this would add. Maybe I underestimate it and you overestimate it. ;) But again, as long as we don't see the need to turn the script into a library, figuring this out, is a waste of time. |
/usr/share/xcb
by default.test
directory.el_client.el
toxelb_gen.el
.That way we can add tests without distributing them, and we'll stoop distributing the code gen script as well. Thoughts?
If this is OK, I'd like to merge #4 without tests, re-do this PR, then add the tests.