Fix getting root dir from project instance

[bitti]

I reinstalled my Emacs 29.0.50 today which apparently fixed some issues, because it seems I had some outdated libraries symlinked. But in return helm-projectile with helm-file-preview-mode enabled got broken with the following message:

let*: Wrong type argument: characterp, git

A little debugging revealed the form (root (cdr (project-current))) as the culprit. Apparently the format of the project instance changed in https://git.savannah.gnu.org/cgit/emacs.git/commit/lisp/progmodes/project.el?id=86969f9658e278ebacb3d625d0309046ff1f2b54. But to my understanding the exact format is an implementation detail anyway which shouldn't be relied on. Instead the generic functions like project-root are supposed to be used, which is what this PR proposes to do.

This should be backwards compatible since the project-root generic is available since the beginning: https://git.savannah.gnu.org/cgit/emacs.git/commit/lisp/progmodes/project.el?id=f8c720b55b9419c849ea9febe6f888761a61949b.

[jcs090218]

It seems like we are getting the following warning in version 26.x and 27.x,

In end of data:
helm-file-preview.el:170:1:Warning: the function `project-root' is not known
    to be defined.

Maybe something is missing, (require 'project), etc?

[bitti]

Maybe something is missing, (require 'project), etc?

But you're already using the project-current function which is part of project and both functions go back to Emacs 25. So I don't see why this should be necessary.

I suppose project-current is autoloaded which is why you don't need an explicit require? But since we call project-current first this should work fine at runtime. Maybe putting in a line like

(declare-function project-root "project")

is enough to resolve the warning? Could you test that, since I don't have Emacs 26/27 available?

[jcs090218]

I have tested project-root, and it's not defined below version 27.2 or lower. We might have to check with emacs-version,

(if (version< "27.2" emacs-version)
    (project-root (project-current))
  (cdr (project-current)))

and of course with,

(declare-function project-root "project")

[bitti]

I have tested project-root, and it's not defined below version 27.2 or lower.

I don't understand that, since it's clearly tagged for Emacs 25:

$ git describe --contains f8c720b55b9419c849ea9febe6f888761a61949b
emacs-25.0.90~1509

We might have to check with emacs-version,

(if (version< "27.2" emacs-version)
    (project-root (project-current))
  (cdr (project-current)))

If we have to do this, I'd rather avoid to check this at runtime to not obscure the code. Why not make a conditional declaration:

(when (version<= emacs-version "27.2")
  (defun project-root (project)
    (cdr project)))

[jcs090218]

I don't understand that, since it's clearly tagged for Emacs 25:

From what I have found, the function project-root isn't there, see their source in branch emacs-25.

Same with branch emacs-27. But you can find it in emacs-28

TBH, I don't know why but the test result seems to be accurate! 😕 (the warning message in the previous comment)

If we have to do this, I'd rather avoid to check this at runtime to not obscure the code. Why not make a conditional declaration:

I second this solution. :)

[bitti]

I don't understand that, since it's clearly tagged for Emacs 25:

From what I have found, the function project-root isn't there, see their source in branch emacs-25.

Ah, now I see where my confusion is coming from. The function was called project-roots beforehand, it was then refactored later to project-root: https://github.com/emacs-mirror/emacs/commit/5044c19001fe608f2eac621add2e05cbca6c804b. To be really correct I suppose we would have to use (car (project-roots (project-current))) for older Emacsen, but we don't have to be holier-than-thou since apparently there were no problems reported for older Emacsen.

If we have to do this, I'd rather avoid to check this at runtime to not obscure the code. Why not make a conditional declaration:

I second this solution. :)

Ok then I push a matching commit.

[bitti]

Now I wonder: since https://github.com/emacs-mirror/emacs/commit/5044c19001fe608f2eac621add2e05cbca6c804b is tagged with emacs-28.0.90 shouldn't we rather check for versions before 28.0.90 to be on the safe side?

[jcs090218]

Yes, please check it with 28.0.90.!

[bitti]

Yes, please check it with 28.0.90.!

Done. My last commit was wrong anyway, since I pushed the wrong code.

[bitti]

Still wrong, it was a long day... Furthermore I still see the warning, but I push the fix first to see if that helps.

[jcs090218]

Yeah, there are two warnings we need to resolve.

In toplevel form:
100
helm-file-preview.el:70:1:Warning: Unused lexical argument ‘project’
101
102
In end of data:
103
helm-file-preview.el:174:1:Warning: the function ‘project-root’ is not known
104
    to be defined.

Sorry, I am on my phone now. The output is with the line number...

[bitti]

Ok, finally I understand why we still get the warning: even though it may be defined at compile time the compiler still throws the warning because it doesn't know if this will always be the case due to the when condition. So I added the declare-function statement. I was worried because we say the function is defined in project.el which is a lie for older Emacsen, but it turns out that isn't even checked unless that is explicitly done via check-declare-file. Another alternative I found to avoid the warning is by using an alias instead: https://emacs.stackexchange.com/a/34327. I'm not sure if I like that better though.

As a bonus I fixed another unrelated warning (c2554c5).

[jcs090218]

Thanks for everything for this PR! :)

Merge this now!