Suggestion: Indent fields according to bibtex.el variables

[bigodel]

Currently, when writing an entry to the .bib file, each entry is formatted with ebib--format-entry. There, right around line 1974, each field is being indented with a \t char, but that does not respect bibtex-field-indentation. My proposed change is the following:

diff -u --label modified-ebib-utils.el ebib-utils.el
--- ebib-utils.el
+++ modified-bib-utils.el
@@ -1971,7 +1971,14 @@
                     (reverse entry)))
       (insert (format "@%s{%s,\n" type key))
       (insert (mapconcat (lambda (field)
-                           (format "\t%s = %s" (car field) (cdr field)))
+                           (format "%s%s = %s"
+                                   (if indent-tabs-mode
+                                       "\t"
+                                     (make-string (+ bibtex-entry-offset
+                                                     bibtex-field-indentation)
+                                                  ? ))
+                                   (car field)
+                                   (cdr field)))
                          entry
                          ",\n"))
       (insert "\n}\n\n"))))

It is a bit ugly, but I just wanted to show how it would work. Notice that adding bibtex-entry-offset to it was taken from some indentation code in bibtex.el.

I could create a PR for this, but it is such a small change that I didn't think it was worth it.

[joostkremers]

The proposal definitely makes sense, but it probably needs to be made customisable, because some people keep their .bib files under version control, and a change like this would cause a big diff with only cosmetic effect. Or more likely, a user would make a small change to their .bib file, but find a humongous diff affecting the entire file.

If we make this customisable, though, we also need to consider what to use as the default setting. If enabled by default, users will still have the same shock when they modify their .bib file, but at least there's an option to undo the effect. Or they can decide to go with it and commit the change.

If it's disabled by default, many users may never realise the option exists...

[bigodel]

Those are indeed valid concerns which I was not considering... Backwards compatibility really is a b****.

I'd say keeping it disabled by default is our only choice for the option, since we don't want to break anyone's previous workflow. We could choose to have it on by default with a message, probably a y-or-n-p prompt, asking if the user really wants to change the whole indentation of the file, but that would require us to write a function to check such a thing -- which I can't seem to think of a 100% reliable way to do so -- and it would most likely annoy a bunch of people, which isn't great :/.

Unfortunately Ebib doesn't seem to have a CHANGELOG file (did I miss it?), otherwise we could just add the entry there to let users know of the new option, which I find more than acceptable. At least that's how I've been rolling with most of the packages I use, many of which have introduced breaking changes that I had to take actions as a user (Citar, for example, has introduced quite a few breaking changes since I started using it).

I would probably go with having a ebib-use-bibtex-indentation option set to t by default, but warning users on the first time they're saving the database (though this is also a bit flimsy, since I can only think of implementing it as an internal variable whose value is saved to custom, but some users don't use a custom file or set it to /dev/null), after they updated the package, about the new change. I would also probably only leave the warning there for a couple of minor versions, e.g. until version 2.5.

[joostkremers]

Those are indeed valid concerns which I was not considering... Backwards compatibility really is a b****.

Yes, it can be annoying sometimes...

Unfortunately Ebib doesn't seem to have a CHANGELOG file (did I miss it?),

There is a NEWS file: https://joostkremers.github.io/ebib/NEWS.html

I have, in fact, introduced breaking changes before, and I think there's a fairly good reason to do it here. I considered the options for introducing a warning that you suggested, but like you said, they are not entirely reliable and may be equally annoying for some users. (Personally, I don't like my custom file being used for that sort of thing...)

Users that keep their .bib files under version control, though, will most likely notice the big change and investigate, and then find out how to undo it.

[bigodel]

There is a NEWS file: https://joostkremers.github.io/ebib/NEWS.html

Oh, I had missed that! Thank you very much.

I have, in fact, introduced breaking changes before, and I think there's a fairly good reason to do it here.

I do think there is a good reason here either, but since no one had ever complained about it I might be mistaken. Honestly, I don't know what would be the best course of action in this particular case, as I'm not familiar with maintaining packages and having to deal with user feedback, so I whatever you think is the best course I will happily comply.

Users that keep their .bib files under version control, though, will most likely notice the big change and investigate, and then find out how to undo it.

Maybe we leave the new suggested variable, ebib-use-bibtex-indentation, as nil by default, keeping it backwards-compatible in order to avoid these sorts of things. Of course, documentation would need to be added.

We could also keep it as nil for a couple of versions, with a warning on the docstring, and then introduce a breaking change in the future if you think that the default behaviour of respecting bibtex's indentation is desired. I have seem a couple of big packages do that already, and it seems like it worked out mostly fine.

[joostkremers]

Well, it's been a year, so high time, I guess... Sorry it took so long. 😞

I made some small changes to your suggested code: the test for indent-tabs-mode is not useful, because Ebib uses a temp buffer to write the database to, which means that we wouldn't get the value the option has in a bibtex-mode buffer anyway. Not an ideal solution, but I'm not sure if there's a better one.

I also moved the code for calculating the indentation to outside the mapconcat. No reason to calculate it on each iteration, after all. 😄

I added a new option, ebib-save-indent-as-bibtex, that's turned off by default. I don't plan to ever change the default, though. That would only make sense if Ebib would check the setting in bibtex-mode, but that would be overkill, I believe. Users that really care about this will probably be able to find the option.

Thanks for making this suggestion and apologies again that it took so long.

[bigodel]

Well, it's been a year, so high time, I guess... Sorry it took so long. 😞

Hey, no worries!! Most of my contributions to open-source software take about a year as well haha. Life gets in the way :P

I made some small changes to your suggested code: the test for `indent-tabs-mode` is not useful, because Ebib uses a temp buffer to write the database to, which means that we wouldn't get the value the option has in a `bibtex-mode` buffer anyway. Not an ideal solution, but I'm not sure if there's a better one.

Doesn't Ebib store any reference to the original buffer? If so, we could just copy the value from there when creating the new buffer.

I also moved the code for calculating the indentation to outside the `mapconcat`. To reason to calculate it on each iteration, after all. 😄

Indeed! Good catch.

Thanks for making this suggestion and apologies again that it took so long.

Thank you for adding the new option. And again, no need to worry about taking long, my workaround has been working for me in the meanwhile.

…

-- João Pedro de A. Paula IT bachelors at Universidade Federal do Rio Grande do Norte (UFRN)

[joostkremers]

Doesn't Ebib store any reference to the original buffer? If so, we could just copy the value from there when creating the new buffer.

No, when Ebib reads a .bib file, it creates a temp buffer to put the contents of the .bib file in. Ebib stores the data in a hash table and the buffer is killed again as soon as the data has been read.

Besides, the temp buffer remains in fundamental-mode, so there's no information on how the user configured bibtex-mode.

[bigodel]

No, when Ebib reads a `.bib` file, it creates a temp buffer to put the contents of the `.bib` file in. Ebib stores the data in a hash table and the buffer is killed again as soon as the data has been read.

Can you point to the piece of code where that is done?

Besides, the temp buffer remains in `fundamental-mode`, so there's no information on how the user configured `bibtex-mode`.

Wouldn't be an issue if we're talking about `indent-tabs-mode` since it isn't `bibtex-mode` specific.

…

-- João Pedro de A. Paula IT bachelors at Universidade Federal do Rio Grande do Norte (UFRN)

[joostkremers]

Can you point to the piece of code where that is done?

Loading of a .bib file is done in ebib--load-bibtex-file-internal, in ebib.el (line 1330 at the time of writing).

Wouldn't be an issue if we're talking about indent-tabs-mode since it isn't bibtex-mode specific.

I'm afraid it is, because indent-tab-mode automatically becomes buffer-local when set, so the global value does not necessarily reflect the value it would have in a bibtex-mode buffer.

[bigodel]

Em sexta, 17/11/2023 às 15:21, Joost Kremers ***@***.***> escreveu:

Loading of a `.bib` file is done in `ebib--load-bibtex-file-internal`, in `ebib.el` (line 1330 at the time of writing).

Tried looking into it real quick but couldn't really grok it. Lately I've not been having much time for it, I'm sorry.

I'm afraid it is, because `indent-tab-mode` automatically becomes buffer-local when set, so the global value does not necessarily reflect the value it would have in a `bibtex-mode` buffer.

Ah, I get it... Just throwing ideas out there, but what if we just created an empty `.bib` buffer, got the value for `indent-tabs-mode` and then continued on with its value? BTW, why are you doing it like that?

…

-- João Pedro de A. Paula IT bachelors at Universidade Federal do Rio Grande do Norte (UFRN)

[joostkremers]

Just throwing ideas out there, but what if we just created an empty .bib buffer, got the value for indent-tabs-mode and then continued on with its value?

Well, you'd really need to open the file itself, because file- and dir-local variables in Emacs make it possible to have different values for indent-tabs-mode for different files and/or directories.

BTW, why are you doing it like that?

I did originally look at bibtex.el to see if it would be useful for parsing .bib files, but I couldn't find any parsing functions in it, just functions that returned the start and end positions of an entry. So I wrote my own parsing code (which I later extracted into its own library, parsebib.el). I used a temp buffer to open a .bib file because IMHO there was no need to run the extra initialisation and possibly hooks that come with bibtex-mode.

Though if we want to support the possibility of having different indentation methods for different .bib files, there is a need to open the files in bibtex-mode.

I'll need to think about that a bit and see if I want to implement it, and if so, how.

[bigodel]

Well, you'd really need to open the file itself, because file- and dir-local variables in Emacs make it possible to have different values for `indent-tabs-mode` for different files and/or directories.

Fair enough.

I did originally look at `bibtex.el` to see if it would be useful for parsing `.bib` files, but I couldn't find any parsing functions in it, just functions that returned the start and end positions of an entry. So I wrote my own parsing code (which I later extracted into its own library, `parsebib.el`).

I guess that's not the case anymore, there are several `bibtex-parse-*` functions now, most notably `bibtex-parse-entry` which returns the entrie's information in an alist very similar to `parsebib-read-entry`.

I used a temp buffer to open a `.bib` file because IMHO there was no need to run the extra initialisation and possibly hooks that come with `bibtex-mode`. Though if we want to support the possibility of having different indentation methods for different `.bib` files, there *is* a need to open the files in `bibtex-mode`.

Yeah, but maybe there is a way to open a buffer and load local variables without actually running the mode, but that would require digging into Emacs' internals a bit. Perhaps disabling things like `font-lock-mode` would be useful as well

I'll need to think about that a bit and see if I want to implement it, and if so, how.

All right, no rush. If I can help with anything please let me know!

…

-- João Pedro de A. Paula IT bachelors at Universidade Federal do Rio Grande do Norte (UFRN)