Splitting image-dired.el into smaller files

Splitting image-dired.el into smaller files

[Stefan Kangas]

Hi emacs-devel (copy to Mathias Dahl),

I'm working on image-dired.el lately, and I have plans for further
improvements that would be pretty far-reaching (if I can get that far).

But it feels at times like treading water, because the organization of
that file is not good.  While it has an okay foundation in terms of the
actual code, it has grown into a small organizational mess internally
over time.

Please consider the section markers I've recently added to that file;
and note that I spent some time thinking about them and where to place
them.  (Hint: good sectioning is currently not doable.)

My current best idea for improving its organization is to split it up in
several files along these lines:

    image/image-dired-bookmarks.el       ;; bookmark.el support
    image/image-dired-compat.el          ;; compatibility layer
    image/image-dired-gallery.el         ;; HTML gallery generation
    image/image-dired-tags.el            ;; home-cooked image tags
    image/image-dired-thumbs.el          ;; thumbnail generation
    image/image-dired.el

The big drawback here is that we will lose the git history.  However, we
lost most of that already in 2007 when the file was renamed.  From my
work and what I've seen so far, I don't think I will miss any history
that won't be massively outweighed by the benefits of better code
organization.

I'd also point out that the alternative to splitting it up into smaller
files would be to move things around in the file, which is also likely
to be pretty bad for git archaeologists.

One important thing to keep in mind when doing this, is to try not to
overdo it.  We don't want a situation where you need to be constantly
grepping to get anything done.  I think this can be avoided if it is
done with care, and considering which parts are orthogonal.  (For
example, the HTML gallery generation really has little to do with
anything else; it is almost entirely its own thing.)

I'd like to raise this here in case people have any feedback on this
plan, or indeed any significant objections.  Thanks in advance.

Re: Splitting image-dired.el into smaller files

[Stefan Monnier]

>     image/image-dired-bookmarks.el       ;; bookmark.el support
>     image/image-dired-compat.el          ;; compatibility layer
>     image/image-dired-gallery.el         ;; HTML gallery generation
>     image/image-dired-tags.el            ;; home-cooked image tags
>     image/image-dired-thumbs.el          ;; thumbnail generation
>     image/image-dired.el

Looking at the current code, the part that deals with bookmarks
seems tiny.  Why do you want to move that into its own file?

[ No particular opinion on the rest.  ]


        Stefan

Re: Splitting image-dired.el into smaller files

[Stefan Kangas]

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

> Looking at the current code, the part that deals with bookmarks
> seems tiny.  Why do you want to move that into its own file?

Mostly because it is independent of everything else, but maybe it is
better to skip that particular one as it would be so small.

> [ No particular opinion on the rest.  ]

Thanks.

Re: Splitting image-dired.el into smaller files

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> My current best idea for improving its organization is to split it up in
> several files along these lines:
>
>     image/image-dired-bookmarks.el       ;; bookmark.el support
>     image/image-dired-compat.el          ;; compatibility layer
>     image/image-dired-gallery.el         ;; HTML gallery generation
>     image/image-dired-tags.el            ;; home-cooked image tags
>     image/image-dired-thumbs.el          ;; thumbnail generation
>     image/image-dired.el
>
> The big drawback here is that we will lose the git history.  However, we
> lost most of that already in 2007 when the file was renamed.  From my
> work and what I've seen so far, I don't think I will miss any history
> that won't be massively outweighed by the benefits of better code
> organization.

I think sounds like a good reorganisation, and image-dired could use
some of that.  (And like Stefan M said, the bookmark stuff seems small
enough to just keep in image-dired.el.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Splitting image-dired.el into smaller files

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 09 Dec 2021 02:05:02 +0100
> Cc: Mathias Dahl <mathias.dahl@gmail.com>, emacs-devel@gnu.org
> 
> Stefan Kangas <stefan@marxist.se> writes:
> 
> > My current best idea for improving its organization is to split it up in
> > several files along these lines:
> >
> >     image/image-dired-bookmarks.el       ;; bookmark.el support
> >     image/image-dired-compat.el          ;; compatibility layer
> >     image/image-dired-gallery.el         ;; HTML gallery generation
> >     image/image-dired-tags.el            ;; home-cooked image tags
> >     image/image-dired-thumbs.el          ;; thumbnail generation
> >     image/image-dired.el
> >
> > The big drawback here is that we will lose the git history.  However, we
> > lost most of that already in 2007 when the file was renamed.  From my
> > work and what I've seen so far, I don't think I will miss any history
> > that won't be massively outweighed by the benefits of better code
> > organization.
> 
> I think sounds like a good reorganisation, and image-dired could use
> some of that.  (And like Stefan M said, the bookmark stuff seems small
> enough to just keep in image-dired.el.)

If you are going to split the file into several ones, please consider
putting them in their own directory.

Re: Splitting image-dired.el into smaller files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> If you are going to split the file into several ones, please consider
> putting them in their own directory.

Thanks, good idea.  Did you have something like "lisp/image-dired" in mind?

Re: Splitting image-dired.el into smaller files

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 9 Dec 2021 14:58:43 +0100
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, Mathias Dahl <mathias.dahl@gmail.com>, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > If you are going to split the file into several ones, please consider
> > putting them in their own directory.
> 
> Thanks, good idea.  Did you have something like "lisp/image-dired" in mind?

Yep.

Re: Splitting image-dired.el into smaller files

[Mathias Dahl]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

Hi all, sorry for coming late to the party, my reading of the list is ...
spotty.

No objections from me on splitting up good old "Tumme.el" :), and thanks
for asking. It was a while since I looked at the full code, but I can
imagine it could be structured better. After all, what started as a few
keyboard macros I needed to quickly organize my photos back then grew into
something I never imagined...

People seem to have made quite a few enhancements and fixes over the years,
some have asked me about it but I suspect people have done silent
maintenance of it as well, and I am very happy for that, and also happy to
see it is still alive and that it might benefit users after so many years.

I also cannot refute feeling some sort of "pride" in having a fellow
countryman doing some heavy lifting on the code base again :-) (as if the
work done by Lars over at our little brother country was not enough... :-p )

Anyway, many thanks for doing work on this.

Go go go!

/Mathias

PS. I have not looked at it for so many years, but I am kind of surprised
to hear the gallery/HTML/generating stuff is still in there. That, if
anything, was a really unfinished hack, but with good intentions of course.
I was most happy with the thumbnail generation and display, and the
generally efficient workflows you could get in browsing and organizing your
pictures. And I liked the tags idea as well...

[-- Attachment #2: Type: text/html, Size: 1621 bytes --]

Renaming files with git not all that bad?

[Stefan Kangas]

Stefan Kangas <stefan@marxist.se> writes:

> The big drawback here is that we will lose the git history.

I tested this just now, and unless I'm missing something, a rename
actually doesn't have to be all that bad.  It turns out that
vc-annotate, magit-blame and vc-region-history have little trouble
handling a rename if its done properly.

In any case, I made a commit containing no other changes than

    git mv lisp/image-dired.el lisp/image

and in the new file, I get history and blame in the new file all the way
back to 2007, when the file was last renamed.

Even vc-print-log works, but I first have to say

    (setq vc-git-print-log-follow t)

though that option is not without its caveats (it skips merge commits).
(See the comment in vc-git.el.)

The key thing seems to be making sure to make no other changes than the
actual moving of the file in a single commit.  This is to make it easy
for the git heuristics to notice that the file was moved.  And then most
of the stuff we care about will work seamlessly, or nearly seamlessly as
seems to be the case with vc-print-log.

It looks like CONTRIBUTE already has the necessary advice here (though I
have no idea why it's talking about creating a feature branch, which
doesn't seem necessary).

I'm not sure if this is new to anyone else than me, but I was positively
surprised with how well git handled this.  I'm not sure if I just never
figured out to do this properly, if git got better, or what.

Re: Renaming files with git not all that bad?

[Phil Sainty]

On 2021-12-09 16:20, Stefan Kangas wrote:
> The key thing seems to be making sure to make no other changes
> than the actual moving of the file in a single commit.  This
> is to make it easy for the git heuristics to notice that the
> file was moved.

That's the right process IMO.  If ever I'm going to rename a
file which has uncommitted changes, I'll stash my changes and
introduce a separate rename commit for the original file content
before continuing, for the exact reason of maximising Git's
ability to detect it.

Git does seem fairly good at calling things a rename even if
I've forgotten to do it separately -- it seems to notice when
the old and the new are very similar, and make the assumption --
but if the old and new files are literally the same then Git
will be dealing with an identical hash for that blob; and so if
a commit is deleting a filename for that blob and also adding
a filename for the identical blob, Git doesn't have to work
very hard to decide that it's a rename!  (For the same reason
I would assume that it's also more efficient to follow renames
when they are done this way).

It should be noted that (IIRC) it isn't *default* behaviour for
Git to follow changes across renames[1], but AFAIK the "--follow"
option is the typical way to ask it to do so, and the likes of
vc and magit can ensure that this is used automatically in cases
where it's necessary.

[1] Not for all commands, at least.  Offhand I know that git
blame does follow renames by default, and there might be others.


-Phil

Re: Renaming files with git not all that bad?

[Andreas Schwab]

On Dez 09 2021, Phil Sainty wrote:

> On 2021-12-09 16:20, Stefan Kangas wrote:
>> The key thing seems to be making sure to make no other changes
>> than the actual moving of the file in a single commit.  This
>> is to make it easy for the git heuristics to notice that the
>> file was moved.
>
> That's the right process IMO.  If ever I'm going to rename a
> file which has uncommitted changes, I'll stash my changes and
> introduce a separate rename commit for the original file content
> before continuing, for the exact reason of maximising Git's
> ability to detect it.

Git doesn't care.  The rename detection is always performed on the two
end points of a diff operation, irrespective of the intervening history.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Renaming files with git not all that bad?

[tomas]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

On Thu, Dec 09, 2021 at 09:43:54AM +0100, Andreas Schwab wrote:
> On Dez 09 2021, Phil Sainty wrote:

[...]

> Git doesn't care.  The rename detection is always performed on the two
> end points of a diff operation, irrespective of the intervening history.

That said, rename detection is based on heuristics and shaped by one or
more configuration knobs. So it is wise to either understand that
heuristics [1] well or to keep rename commits somewhat "isolated"
(personally, I tend to not make (biggish) changes in the file(s) I'm
renaming in the same commit. YMMV, heuristically, of course.

I also have acquired the custom to use "git mv" and friends, be it to
remind myself that I'm doing it under VC (and to keep that peripheral
eye to whether VC is doing what I think it does).

Cheers

[1] some similarity measure plus some threshold. There are also rumours
   of massive renames hitting some limits.

-- 
tomás

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Renaming files with git not all that bad?

[Eli Zaretskii]

> Date: Thu, 09 Dec 2021 16:56:05 +1300
> From: Phil Sainty <psainty@orcon.net.nz>
> Cc: Mathias Dahl <mathias.dahl@gmail.com>, emacs-devel@gnu.org
> 
> Git does seem fairly good at calling things a rename even if
> I've forgotten to do it separately -- it seems to notice when
> the old and the new are very similar, and make the assumption --
> but if the old and new files are literally the same then Git
> will be dealing with an identical hash for that blob; and so if
> a commit is deleting a filename for that blob and also adding
> a filename for the identical blob, Git doesn't have to work
> very hard to decide that it's a rename!  (For the same reason
> I would assume that it's also more efficient to follow renames
> when they are done this way).

The problem is not with deciding a change is a rename, the problem is
with following the rename in a way that is useful for whatever Git
command you are invoking.

> It should be noted that (IIRC) it isn't *default* behaviour for
> Git to follow changes across renames[1], but AFAIK the "--follow"
> option is the typical way to ask it to do so, and the likes of
> vc and magit can ensure that this is used automatically in cases
> where it's necessary.

Some Git commands don't have the --follow switch, AFAIR, or have it in
a way that forbids using other useful options.  So renaming still has
its downsides, albeit they aren't catastrophic nor affect every single
Git feature.

Re: Renaming files with git not all that bad?

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> Some Git commands don't have the --follow switch, AFAIR, or have it in
> a way that forbids using other useful options.  So renaming still has
> its downsides, albeit they aren't catastrophic nor affect every single
> Git feature.

Right.  It seems a whole lot better than what was my previous
understanding, at least, but I'm not sure how it affects our current
strong inclination to go out of our way to avoid renames.

Re: Renaming files with git not all that bad?

[Yuri Khan]

On Thu, 9 Dec 2021 at 10:20, Stefan Kangas <stefan@marxist.se> wrote:

> > The big drawback here is that we will lose the git history.
>
> I tested this just now, and unless I'm missing something, a rename
> actually doesn't have to be all that bad.  It turns out that
> vc-annotate, magit-blame and vc-region-history have little trouble
> handling a rename if its done properly.
>
> In any case, I made a commit containing no other changes than
>
>     git mv lisp/image-dired.el lisp/image

There is also a trick to split a file into multiple parts where each
part retains its history.

https://devblogs.microsoft.com/oldnewthing/20190916-00/?p=102892

The gist of it is:

* From the original point, start multiple branches, one for each part.
* On each branch:
  * Rename the original file to the file name of the target part, and
commit just this renaming.
  * Then, remove the lines that do not belong to this part, and also commit.
* Finally, do an octopus merge on all the branches together.

This way, each part traces its ancestry to the original file through a
rename-only commit.

Re: Renaming files with git not all that bad?

[Tassilo Horn]

Yuri Khan <yuri.v.khan@gmail.com> writes:

> The gist of it is:
>
> * From the original point, start multiple branches, one for each part.
> * On each branch:
>   * Rename the original file to the file name of the target part, and
> commit just this renaming.
>   * Then, remove the lines that do not belong to this part, and also commit.
> * Finally, do an octopus merge on all the branches together.
>
> This way, each part traces its ancestry to the original file through a
> rename-only commit.

Couldn't the same effect be achieved in a simpler manner by copying the
original file N times in one commit and then stripping the copies and
original down to what they should eventually become?  (AFAIK, git has no
problem detecting literal copies.)

Bye,
Tassilo

Re: Renaming files with git not all that bad?

[Yuri Khan]

On Thu, 9 Dec 2021 at 13:06, Tassilo Horn <tsdh@gnu.org> wrote:

> Couldn't the same effect be achieved in a simpler manner by copying the
> original file N times in one commit and then stripping the copies and
> original down to what they should eventually become?  (AFAIK, git has no
> problem detecting literal copies.)

Indeed, I tried this and it works for me, as long as the first commit
is only literal copies. Maybe Git’s ancestry detection through copies
was not as advanced in the unspecified times when Raymond invented his
technique.

Re: Renaming files with git not all that bad?

[Tassilo Horn]

Yuri Khan <yuri.v.khan@gmail.com> writes:

>> Couldn't the same effect be achieved in a simpler manner by copying
>> the original file N times in one commit and then stripping the copies
>> and original down to what they should eventually become?  (AFAIK, git
>> has no problem detecting literal copies.)
>
> Indeed, I tried this and it works for me, as long as the first commit
> is only literal copies.  Maybe Git’s ancestry detection through copies
> was not as advanced in the unspecified times when Raymond invented his
> technique.

Yes, that might be.  BTW, if you still have your test example: what
happens when you squash the copy commit and the following strip-down
commits?  Is the history still intact?  (I guess, no, but who knows.)

Bye,
Tassilo

Re: Renaming files with git not all that bad?

[Yuri Khan]

On Thu, 9 Dec 2021 at 14:21, Tassilo Horn <tsdh@gnu.org> wrote:
> Yes, that might be.  BTW, if you still have your test example: what
> happens when you squash the copy commit and the following strip-down
> commits?  Is the history still intact?  (I guess, no, but who knows.)

With a squash, the stripped-down copies are different enough from the
original file to not be recognized as descendants. The diff is shown
as a single file deletion and creation of three new files.

For ancestry detection to work 100% reliably, the descendant file must
have exactly the same hash as the ancestor file, recorded in a commit.

Re: Renaming files with git not all that bad?

[Stefan Kangas]

Yuri Khan <yuri.v.khan@gmail.com> writes:

>> Couldn't the same effect be achieved in a simpler manner by copying the
>> original file N times in one commit and then stripping the copies and
>> original down to what they should eventually become?  (AFAIK, git has no
>> problem detecting literal copies.)
>
> Indeed, I tried this and it works for me, as long as the first commit
> is only literal copies. Maybe Git’s ancestry detection through copies
> was not as advanced in the unspecified times when Raymond invented his
> technique.

I'm having no luck with this.

Could you describe the exact steps you took?

Re: Renaming files with git not all that bad?

[Tassilo Horn]

Stefan Kangas <stefan@marxist.se> writes:

Hi Stefan,

>>> Couldn't the same effect be achieved in a simpler manner by copying the
>>> original file N times in one commit and then stripping the copies and
>>> original down to what they should eventually become?  (AFAIK, git has no
>>> problem detecting literal copies.)
>>
>> Indeed, I tried this and it works for me, as long as the first commit
>> is only literal copies. Maybe Git’s ancestry detection through copies
>> was not as advanced in the unspecified times when Raymond invented
>> his technique.
>
> I'm having no luck with this.
>
> Could you describe the exact steps you took?

I've just tried:

  git checkout -b copy-test
  cp lisp/image-dired.el lisp/image-dired-1.el
  cp lisp/image-dired.el lisp/image-dired-2.el
  git add lisp/image-dired-*.el
  git commit -m "copied image-dired.el twice"

Thereafter,

  git log -- lisp/image-dired-1.el lisp/image-dired-2.el

show only my "copied image-dired.el twice" commit unless I also specify
--follow.

I've also tried the "multiple renames on different branches followed by
an octopus merge" strategy but the result looks identical.

Oh, no, not quite.  It's the same wrt. "git log" but the difference is
that with the copy strategy, "git blame" will show you as having touched
all lines during the copy (which is sensible somehow) whereas they keep
the original previous last authors with the renaming strategy.

One problem with the renaming strategy is that the original file is
inevitably renamed after the merge, or at least you have to have one
branch in the merge where it is modified in the hope to get a conflict
so that you can keep all renamed versions and the original.  However,
since you want to move all image-dired related files into their own
directory, that's no problem whatsoever.

Bye,
Tassilo

Re: Renaming files with git not all that bad?

[Yuri Khan]

On Fri, 10 Dec 2021 at 05:03, Stefan Kangas <stefan@marxist.se> wrote:

> >> Couldn't the same effect be achieved in a simpler manner by copying the
> >> original file N times in one commit and then stripping the copies and
> >> original down to what they should eventually become?  (AFAIK, git has no
> >> problem detecting literal copies.)
> >
> > Indeed, I tried this and it works for me, as long as the first commit
> > is only literal copies. Maybe Git’s ancestry detection through copies
> > was not as advanced in the unspecified times when Raymond invented his
> > technique.
>
> I'm having no luck with this.
>
> Could you describe the exact steps you took?

I start with the same steps described in Raymond Chen’s article:

git init

>foods echo apple
>>foods echo celery
>>foods echo cheese
git add foods
git commit --author="Alice <alice>" -m created

>>foods echo eggs
>>foods echo grape
>>foods echo lettuce
git commit --author="Bob <bob>"   -am middle

>>foods echo milk
>>foods echo orange
>>foods echo peas
git commit --author="Carol <carol>" -am last

git tag ready


Next:

cp foods dairy
cp foods fruits
git mv foods veggies
git add dairy fruits
git commit --author="Donald <donald>" -m "split part 1"

sed -rni '/^a|^g|^o/p' fruits
sed -rni '/^ce|^l|^p/p' veggies
sed -rni '/^ch|^e|^m/p' dairy
git commit --author="Donald <donald>" -am "split part 2"

At this point, the line history in each file is preserved:

git blame dairy
^2221a21 foods (Alice 2021-12-09 13:27:21 +0700 1) cheese
d12be427 foods (Bob   2021-12-09 13:27:27 +0700 2) eggs
ada8f5c5 foods (Carol 2021-12-09 13:27:31 +0700 3) milk

git blame fruits
^2221a21 foods (Alice 2021-12-09 13:27:21 +0700 1) apple
d12be427 foods (Bob   2021-12-09 13:27:27 +0700 2) grape
ada8f5c5 foods (Carol 2021-12-09 13:27:31 +0700 3) orange

git blame veggies
^2221a21 foods (Alice 2021-12-09 13:27:21 +0700 1) celery
d12be427 foods (Bob   2021-12-09 13:27:27 +0700 2) lettuce
ada8f5c5 foods (Carol 2021-12-09 13:27:31 +0700 3) peas

‘git log’, however, does not trace file history beyond the split by default:

git log --oneline dairy
8d6d788 (HEAD -> master) split part 2
4be5ed0 split part 1

It does if you ask:

git log --oneline --follow veggies
8d6d788 (HEAD -> master) split part 2
4be5ed0 split part 1
ada8f5c (tag: ready, split-via-rename) last
d12be42 middle
2221a21 created


Doing it Raymond’s way (via multiple branches with a single rename and
paring down on each branch, then octopus-merging the branches)
produces the same effects: blame assigned correctly, file-filtered log
complete only on --follow.

Re: Renaming files with git not all that bad?

[Stefan Kangas]

Yuri Khan <yuri.v.khan@gmail.com> writes:

> There is also a trick to split a file into multiple parts where each
> part retains its history.

Thanks, I didn't know about this but will see if I can make it work
for the splitting up of image-dired.el.

Re: Renaming files with git not all that bad?

[tomas]

[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]

On Thu, Dec 09, 2021 at 03:55:34PM +0100, Stefan Kangas wrote:
> Yuri Khan <yuri.v.khan@gmail.com> writes:
> 
> > There is also a trick to split a file into multiple parts where each
> > part retains its history.
> 
> Thanks, I didn't know about this but will see if I can make it work
> for the splitting up of image-dired.el.

Based on [1], I once did:

  #+file: ~/bin/gitlib
  #+begin_src sh
    #!/bin/bash
    set -ex
    # FIXME sanity checks missing!
    # See git-sh-setup on some helpers for this
    if [ $# -lt 2 -o $# -gt 3 ] ; then
      echo "Usage: $0 orig copy [commit-message]"
      exit 1
    fi
    
    orig="$1"
    copy="$2"
    message=${3:-"split off $copy from $orig"}
    
    if [ -e "$copy" ] ; then
      echo "$copy already exists: bailing out"
      exit 1
    fi
    if [ ! -f "$orig" ] ; then
      echo "$orig isn't an existing file: bailing out"
      # FIXME might want to check that it /is/ a git file
      exit 1
    fi
    
    echo "$message"
    
    git mv "$orig" "$copy"
    git commit -nm "$message"
    REV=$(git rev-parse HEAD)
    git reset --hard HEAD^
    TMP=$(mktemp tmp-XXXXXX)
    # note that $TMP exists after the mktemp, thus -f
    git mv -f "$orig" "$TMP"
    git commit -nm "$message"
    # this fails because of conflicts: expected
    git merge "$REV" || true
    git commit -anm "$message"
    git mv "$TMP" "$orig"
    git commit -nm "$message"
  #+end_src

This is based on two things:
(1) if you happen to have a `git-foo' somewhere in your path, your git
   magically acquires a `foo' subcommand.
(2) I have ~/bin/libgit in my PATH.

It may be rough around the edges. It leaves a "smoke ring" in your
history -- I didn't try to conceal that (basically, it bifurcates the
repo, creating one version with the "first half" of a split and the
other with the "second half" -- which actually, at this point are just
copies under different names, and then merges this bifurcation):

  #+begin_quote
    * fb02315 (HEAD -> master) split-off keywords
    *   07fcba3 split-off keywords
    |\
    * | fb9944d split-off keywords
    | * 44b4849 split-off keywords
    |/
    * 8160976 Pass an expected failure
    * 86107d9 bulk-change all tests to our new test func
  #+end_quote

After having done all of that, I somehow decided that it ain't worth the
hassle and have used it very little. So please, use with care!

Cheers

[1] https://stackoverflow.com/questions/3887736/keep-git-history-when-splitting-a-file/53849651#53849651

-- 
t

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Renaming files with git not all that bad?

[Stefan Kangas]

<tomas@tuxteam.de> writes:

> On Thu, Dec 09, 2021 at 03:55:34PM +0100, Stefan Kangas wrote:
>> Yuri Khan <yuri.v.khan@gmail.com> writes:
>>
>> > There is also a trick to split a file into multiple parts where each
>> > part retains its history.
>>
>> Thanks, I didn't know about this but will see if I can make it work
>> for the splitting up of image-dired.el.
>
> Based on [1], I once did:
>
>   #+file: ~/bin/gitlib
>   #+begin_src sh
>     #!/bin/bash
>     set -ex
>     # FIXME sanity checks missing!
>     # See git-sh-setup on some helpers for this
>     if [ $# -lt 2 -o $# -gt 3 ] ; then
>       echo "Usage: $0 orig copy [commit-message]"
>       exit 1
>     fi
>
>     orig="$1"
>     copy="$2"
>     message=${3:-"split off $copy from $orig"}
>
>     if [ -e "$copy" ] ; then
>       echo "$copy already exists: bailing out"
>       exit 1
>     fi
>     if [ ! -f "$orig" ] ; then
>       echo "$orig isn't an existing file: bailing out"
>       # FIXME might want to check that it /is/ a git file
>       exit 1
>     fi
>
>     echo "$message"
>
>     git mv "$orig" "$copy"
>     git commit -nm "$message"
>     REV=$(git rev-parse HEAD)
>     git reset --hard HEAD^
>     TMP=$(mktemp tmp-XXXXXX)
>     # note that $TMP exists after the mktemp, thus -f
>     git mv -f "$orig" "$TMP"
>     git commit -nm "$message"
>     # this fails because of conflicts: expected
>     git merge "$REV" || true
>     git commit -anm "$message"
>     git mv "$TMP" "$orig"
>     git commit -nm "$message"
>   #+end_src

Cool!

I'm not sure about this method, as it seems safer to use a named branch
instead of referring to some SHA1 directly.  Maybe that won't make any
difference to git though?

It's been over a decade since I last looked into how git stores data in
any detail, but IIRC a branch name is just a pointer to some SHA1 hash
recorded in a text file.

Re: Renaming files with git not all that bad?

[tomas]

[-- Attachment #1: Type: text/plain, Size: 1609 bytes --]

On Thu, Dec 09, 2021 at 02:18:21PM -0800, Stefan Kangas wrote:
> <tomas@tuxteam.de> writes:
> 
> > On Thu, Dec 09, 2021 at 03:55:34PM +0100, Stefan Kangas wrote:
> >> Yuri Khan <yuri.v.khan@gmail.com> writes:
> >>
> >> > There is also a trick to split a file into multiple parts where each
> >> > part retains its history.
> >>
> >> Thanks, I didn't know about this but will see if I can make it work
> >> for the splitting up of image-dired.el.
> >
> > Based on [1], I once did:

[crude git-split code]

> Cool!
> 
> I'm not sure about this method, as it seems safer to use a named branch
> instead of referring to some SHA1 directly.  Maybe that won't make any
> difference to git though?

It would at least leave some evidence behind, should things go boom :)
OTOH you'd want to garbage collect those spurious branches
> 
> It's been over a decade since I last looked into how git stores data in
> any detail, but IIRC a branch name is just a pointer to some SHA1 hash
> recorded in a text file.

More or less. The "active" branches (typically the HEAD, but also the
remote branches) are represented by files whithin
.git/refs/heads/<branch name> (where subdirs are made when the branch
name contains slashes, that's why you can't create a branch "foo"
whenever you already have a branch "foo/bar" and vice-versa, just think
UNIXy ;-) This file just contains the Merkle-tree-ish hash of the source
tree, and is updated on commits (thus it "grows"). Tags just stay "in
place". In real life, the inactive refs tend to live in "packed-refs", I
believe.

Cheers
-- 
t

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Splitting image-dired.el into smaller files

[Stefan Kangas]

Stefan Kangas <stefan@marxist.se> writes:

> I'm working on image-dired.el lately, and I have plans for further
> improvements that would be pretty far-reaching (if I can get that far).
[...]
> My current best idea for improving its organization is to split it up in
> several files along these lines:

I finally got around to this, and have pushed the scratch/image-dired
branch to Savannah.  Any feedback is much appreciated.

Here is the file organization I settled on, but obviously there is time
to change it before it goes to master:

  image-dired/image-dired-compat.el
  image-dired/image-dired-dired.el
  image-dired/image-dired-external.el
  image-dired/image-dired-gallery.el
  image-dired/image-dired-tags.el
  image-dired/image-dired-util.el
  image-dired/image-dired.el

I'm thinking about whether or not we should just obsolete the HTML
gallery generation stuff (image-dired-gallery.el).  The image-dired.el
author Mathias Dahl said in January that he is "kind of surprised to
hear the gallery/HTML/generating stuff is still in there" and that it
"if anything, was a really unfinished hack".[1]

Footnotes:
[1]  https://mail.gnu.org/r/emacs-devel/2022-01/msg01724.html

Re: Splitting image-dired.el into smaller files

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sat, 20 Aug 2022 17:56:52 -0700
> Cc: Mathias Dahl <mathias.dahl@gmail.com>
> 
>   image-dired/image-dired-compat.el
>   image-dired/image-dired-dired.el
>   image-dired/image-dired-external.el
>   image-dired/image-dired-gallery.el
>   image-dired/image-dired-tags.el
>   image-dired/image-dired-util.el
>   image-dired/image-dired.el

Having "image-dired" both in the directory name and in the file names
sounds redundant to me.

Also, are you sure we need so many separate files?  image-dired.el is
only 3K lines.

Re: Splitting image-dired.el into smaller files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>>   image-dired/image-dired-compat.el
>>   image-dired/image-dired-dired.el
>>   image-dired/image-dired-external.el
>>   image-dired/image-dired-gallery.el
>>   image-dired/image-dired-tags.el
>>   image-dired/image-dired-util.el
>>   image-dired/image-dired.el
>
> Having "image-dired" both in the directory name and in the file names
> sounds redundant to me.

Hmm.  Any ideas for how we could name the files instead?

> Also, are you sure we need so many separate files?  image-dired.el is
> only 3K lines.

If there are no protests to obsoleting the HTML gallery support, we can
remove "image-dired-gallery.el".  We could also easily make do without
"image-dired-compat.el".

Re: Splitting image-dired.el into smaller files

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 21 Aug 2022 07:32:10 -0700
> Cc: emacs-devel@gnu.org, mathias.dahl@gmail.com
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >>   image-dired/image-dired-compat.el
> >>   image-dired/image-dired-dired.el
> >>   image-dired/image-dired-external.el
> >>   image-dired/image-dired-gallery.el
> >>   image-dired/image-dired-tags.el
> >>   image-dired/image-dired-util.el
> >>   image-dired/image-dired.el
> >
> > Having "image-dired" both in the directory name and in the file names
> > sounds redundant to me.
> 
> Hmm.  Any ideas for how we could name the files instead?

Remove the "image-dired-" part?

> > Also, are you sure we need so many separate files?  image-dired.el is
> > only 3K lines.
> 
> If there are no protests to obsoleting the HTML gallery support, we can
> remove "image-dired-gallery.el".  We could also easily make do without
> "image-dired-compat.el".

And maybe leave the -util.el part inside the main file?

Re: Splitting image-dired.el into smaller files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> Remove the "image-dired-" part?

Do you mean naming the files like this?

    image-dired/compat.el
    image-dired/dired.el
    image-dired/external.el
    image-dired/gallery.el
    image-dired/tags.el
    image-dired/util.el
    image-dired/image-dired.el

How would I then require them, is it like this?

    (require 'image-dired/compat)

If that works, it does sound a bit neater.

> And maybe leave the -util.el part inside the main file?

That's not very easy to do without introducing circular dependencies,
unfortunately.  I thought about autoloading, but then we'd have a bunch
of mainly internal image-dired things autoloaded that I don't think we
really want.

Re: Splitting image-dired.el into smaller files

[Lars Ingebrigtsen]

Stefan Kangas <stefankangas@gmail.com> writes:

> How would I then require them, is it like this?
>
>     (require 'image-dired/compat)
>
> If that works, it does sound a bit neater.

That makes jumping to the files using the normal tools difficult, so I'd
rather just keep the image-dired-foo.el names.  It's what we do in all
other packages.  (Except that we sometimes shorten it, so they could be
imdir-foo, but...)

Re: Splitting image-dired.el into smaller files

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 21 Aug 2022 08:11:08 -0700
> Cc: emacs-devel@gnu.org, mathias.dahl@gmail.com
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Remove the "image-dired-" part?
> 
> Do you mean naming the files like this?
> 
>     image-dired/compat.el
>     image-dired/dired.el
>     image-dired/external.el
>     image-dired/gallery.el
>     image-dired/tags.el
>     image-dired/util.el
>     image-dired/image-dired.el

Yes.

> How would I then require them, is it like this?
> 
>     (require 'image-dired/compat)
> 
> If that works, it does sound a bit neater.

It does work: we use it in cedet/ subdirectories.

> > And maybe leave the -util.el part inside the main file?
> 
> That's not very easy to do without introducing circular dependencies,
> unfortunately.

How come, when all the code now lives on the same file and we don't
have any problems?  Maybe your factoring between the files needs
revisiting?

Re: Splitting image-dired.el into smaller files

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> It does work: we use it in cedet/ subdirectories.

And it's an absolute pain when debugging.

Re: Splitting image-dired.el into smaller files

[Stefan Monnier]

>> Do you mean naming the files like this?
>> 
>>     image-dired/compat.el
>>     image-dired/dired.el
>>     image-dired/external.el
>>     image-dired/gallery.el
>>     image-dired/tags.el
>>     image-dired/util.el
>>     image-dired/image-dired.el
>
> Yes.

image-dired.el is not terribly large, so maybe keeping it as a single
file is not a bad option either.  I think the main goal in splitting
files is when that lets us load only some of them in common use cases.

>> How would I then require them, is it like this?
>>
>>     (require 'image-dired/compat)
>>
>> If that works, it does sound a bit neater.
>
> It does work: we use it in cedet/ subdirectories.

Indeed.  It does suffer from various issues, tho (e.g. `load-history`
doesn't remember that we used `image-dired/tags` to load the file so
(eval-after-load "tags" ...) may run unexpectedly).  It might be good to
use it more widely to force us to face the problems and fix them,
of course.

>> > And maybe leave the -util.el part inside the main file?
>> That's not very easy to do without introducing circular dependencies,
>> unfortunately.

I think most multi-file projects ends up using either ugly/broken hacks
(e.g. ediff and viper), or a package-specific autoloads file for that
(e.g. `mh-autoloads`, `cl-autoloads`, `tramp-autoloads`, ...).

Lars's new autoloads code supports those autoloads file much better than
the old one.


        Stefan

Re: Splitting image-dired.el into smaller files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> How come, when all the code now lives on the same file and we don't
> have any problems?  Maybe your factoring between the files needs
> revisiting?

For example, `image-dired-dir' is used in image-dired-tags.el,
image-dired-dired.el, and image-dired.el.  To have a non-circular
dependency graph, I introduced image-dired-util.el, which gives me a
diamond shaped dependency graph with arrows pointing in only one
direction.

It's possible that this is fixable using autoloads, but that leaves us
with the same number of files, so I'm not sure it's much of an
improvement.

But maybe I'm missing something.

Re: Splitting image-dired.el into smaller files

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Tue, 23 Aug 2022 12:02:47 -0700
> Cc: emacs-devel@gnu.org, mathias.dahl@gmail.com
> 
> For example, `image-dired-dir' is used in image-dired-tags.el,
> image-dired-dired.el, and image-dired.el.  To have a non-circular
> dependency graph, I introduced image-dired-util.el, which gives me a
> diamond shaped dependency graph with arrows pointing in only one
> direction.
> 
> It's possible that this is fixable using autoloads, but that leaves us
> with the same number of files, so I'm not sure it's much of an
> improvement.
> 
> But maybe I'm missing something.

Yes, I think this is the wrong way of dividing the big file into
several smaller ones.  You don't go by dependencies, you go by
functionalities.  And if there's no way of breaking it into separate
functionalities, it means it shouldn't be broken at all, unless
completely rewritten.

The file is just 3K lines, so if it doesn't lend itself easily to
division, it isn't worth the effort, IMO.

Re: Splitting image-dired.el into smaller files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> Yes, I think this is the wrong way of dividing the big file into
> several smaller ones.  You don't go by dependencies, you go by
> functionalities.

If I understand you correctly, that's what I've done:

   image-dired-dired.el
   image-dired-external.el
   image-dired-tags.el
   image-dired.el

I don't currently see how dividing "by functionality" helps with
image-dired-util.el, however.  image-dired-*.el still has to build.
I also fear that the overall structure will be worse if we go out of our
way just to get rid of one file.

OTOH, I note that Gnus, Viper, MH-E, URL, Eshell, etc. all have some
util-file, so the idea isn't completely new or foreign to Emacs.

If there are no better ideas for how to structure this, I'm personally
happy with what is currently on scratch/image-dired.  But ideas are of
course still welcome, and I especially welcome concrete ones.

Re: Splitting image-dired.el into smaller files

[Juri Linkov]

> Here is the file organization I settled on, but obviously there is time
> to change it before it goes to master:
>
>   image-dired/image-dired-compat.el
>   image-dired/image-dired-dired.el
>   image-dired/image-dired-external.el
>   image-dired/image-dired-gallery.el
>   image-dired/image-dired-tags.el
>   image-dired/image-dired-util.el
>   image-dired/image-dired.el

Instead of the subdir 'image-dired/' I recommend to use 'image/'
to have a common name for more image-related packages:

    image/idired-compat.el
    image/idired-dired.el
    image/idired-external.el
    image/idired-gallery.el
    image/idired-tags.el
    image/idired-util.el
    image/idired.el

Re: Splitting image-dired.el into smaller files

[Stefan Kangas]

Juri Linkov <juri@linkov.net> writes:

> Instead of the subdir 'image-dired/' I recommend to use 'image/'
> to have a common name for more image-related packages:

I'm fine with putting this all under 'image/', but I don't think I have
any real opinion either way.  ISTR that Eli was the first to suggest
putting this under 'image-dired/', so maybe he feels differently.

>     image/idired-compat.el
>     image/idired-dired.el
>     image/idired-external.el
>     image/idired-gallery.el
>     image/idired-tags.el
>     image/idired-util.el
>     image/idired.el

idired is not a bad prefix, and it has some precedent in iimage.el.
OTOH, "image-dired" is not too bad either, and it is a bit more
descriptive.

With the completion framework I use, I can type `C-x p f i dir ut RET'
to visit "image-dired-util.el".  I would assume the story is similar
with other completion frameworks (including the default).  So maybe
using "image-dired-*.el" is okay, since the shorter one doesn't really
(or shouldn't really) save any typing.

Re: Splitting image-dired.el into smaller files

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Mon, 22 Aug 2022 06:08:24 -0700
> Cc: emacs-devel@gnu.org, Mathias Dahl <mathias.dahl@gmail.com>
> 
> Juri Linkov <juri@linkov.net> writes:
> 
> > Instead of the subdir 'image-dired/' I recommend to use 'image/'
> > to have a common name for more image-related packages:
> 
> I'm fine with putting this all under 'image/', but I don't think I have
> any real opinion either way.  ISTR that Eli was the first to suggest
> putting this under 'image-dired/', so maybe he feels differently.

I don't mind having those under image/, no.

> idired is not a bad prefix, and it has some precedent in iimage.el.
> OTOH, "image-dired" is not too bad either, and it is a bit more
> descriptive.

Agreed.