Closed minad closed 9 months ago
minad
commented
11 months ago 
amno1
commented
10 months ago font-lock and wdired must not be required
Does that matter? I think I used some internal font-lock stuff (not autolaoded) in earlier version, so that is left since then, but it should be just a simple feature lookup, it shouldn't matter. I have to load wdired to advice functions. Can I advice them before wdired is loaded?
Use fewer advices.
Which one do you have in mind there?
There are problems when switching to and from wdired, emacs will get confused, so I have to ensure that the Readme file is removed before editing a directory. Code in Dired itself reverts buffer when you create a directory or file, I don't remember axactly longer. All advices and hooks used were those I needed to make it work without crashing or visual artifacts. You have to test to create a file, insert a directory, a directory content, dired-subtree, etc. I am rather afraid I have missed some tbh.
The usage of buffer-invisibility-spec seems suspicious. Could you please add an explanatory comment?
I am trying to get org-links to display prettified. In the older version, I used a different method, but in this one, I tried to make it more configurable and smaller, but for some reason, org-mode refuses to display pretty links. If you have a solution I am happy to hear :). The spec is left from my experimenting I think.
Thank you for looking at it. I appreciate all the suggestions.
Edit:
Forgot about with-silent-modifications. I am not sure it was so in all versions tbh. I think it was mixing with undo in some previous version, I don't remember longer. I did add undo restoration because it was modified; I wasn't just guessing it so to say, but I don't remember it longer. My Lisp was really bad back in time too, so I might have just done something else wrong and misunderstood it :-).
minad
commented
9 months ago Arthur Miller @.***> writes:
font-lock and wdired must not be required
Does that matter? I think I used some internal font-lock stuff (not autolaoded) in earlier version, so that is left since then, but it should be just a simple feature lookup, it shouldn't matter. I have to load wdired to advice functions. Can I advice them before wdired is loaded?
font-lock is preloaded, see loadup.el. It doesn't matter to still require the features, but it is a little code smell. You can advise functions before loading them. As soon as the symbol becomes available, the advice will be applied.
Use fewer advices.
Which one do you have in mind there?
Oh, I don't know, but I am generally wary if packages add many advices. This is a sign that the package might be brittle.
The usage of buffer-invisibility-spec seems suspicious. Could you please add an explanatory comment?
I am trying to get org-links to display prettified. In the older version, I used a different method, but in this one, I tried to make it more configurable and smaller, but for some reason, org-mode refuses to display pretty links. If you have a solution I am happy to hear :). The spec is left from my experimenting I think.
As far as I've understood you only use the invisibility spec to find the position of the readme, in order to remove it when needed. You could instead use a simple custom text property to mark the start of the readme.
amno1
commented
9 months ago I am not sure, how long was font-lock preloaded, since which version? I am not sure how far back is it meaningful to keep the backward compatibility. I have just tested it in my Emacs, it works fine to remove but I run a fork of master. If font-lock is preloaded somewhere to say 27 I am ok ot remove the require statement; same for wdired. To be honest that ´'s not a big deal, requiring an already loaded file is cheap, but sure if we can skip a symbol lookup and a funcall it is always welcome.
For the advised functions, I believe this is the least hassle and most efficient way to ensure we are running our function here. If those advised functions were hooks it would be much better, but they are not. I was playing with before/after change hooks, but due to how they have implemented Dired that didn't work. We have to ensure a clean Dired buffer for any function that works on the entire content of Dired buffer. Alternative is do define a custom defun for each function and manage a map of those for interactive use, but than you still have lisp code that calls those functions. Advice here ensures we are calling our code regardless. You can also see this issue: https://github.com/clemera/dired-git-info/issues/9.
That is a fundamental problem due to Emacs assumption that there is only one major mode active in entire buffer. I did have thoughts to use indirect buffers and experimented some time ago with them, but I didn't got it to work back then. I am thinking of giving it a try again, but it is not my priority. If you are interested, please feel free to fork or implement something from scratch, Another thought I have is to scratch this and implement similar on top of Polymode, but it takes a bit against introducing such a big dependency since this is barely 150 SLOC.
For the ivisible-spec, dired-auto-readme does not use it at all so to say. That was just for org-mode links. Since I have changed the code to not use my old hardcoded code to render org-mode files, links are not prettified. I suspect it is some buffer local variable, and was hunting which one was it, but I didn't found it. I have remived that now, I just haven't pushed the patch yet; we are away so I am on my wifes laptop and somehow git and ssh does not seem to be friends with github suddenly.
For the record, dired-auto-readme uses just a single text property to mark the end point of a dired buffer. Since dired buffer is dynamic, files and directories can be inserted, removed, etc, we have to keep track of where the end is, so we can remove the correct portion of the buffer (the readme part) when dired has to run code that operates on the entire buffer. We can't use dired code for this since it has no idea what the inserted text is. We would need major modes to work as region modes, but that would be another discussion.
Edit: Sorry I see it was a bit hasty answer, lots of typos and I am somewhat jumping over the stuff, I just wrote as the thoughts came. If something is unclear ask, I can clarify further if needed.