Skip to content

Recents refactor - #602

Open
ricardoricho wants to merge 5 commits into
emacs-dashboard:masterfrom
ricardoricho:recents-refactor
Open

Recents refactor#602
ricardoricho wants to merge 5 commits into
emacs-dashboard:masterfrom
ricardoricho:recents-refactor

Conversation

@ricardoricho

Copy link
Copy Markdown
Member

This is a refactor of the "recents" section. It changes the way we keep a copy of recentf-list before we visit all the files involved in the construction of the different sections and then restore the list to remove all the pollution of the construction.

One part I'm not sure of is that if recentf-mode is not enable, do we have to
enable it? In the current implementation we are activating the mode to read the
list, but there is recetf-load-list that do that without activating the mode.

I added some test just for this section, but I expect that this will be the
pattern, to have test files for each section and test some features instead of
testing each function. What do you think?

Comment thread test/activate.el Outdated
:load-path "./"
:demand t)
(with-current-buffer (dashboard-open)
(should (string-match-p "Welcome to Emacs!" (buffer-string)))))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would need to manually install use-package for early Emacs versions.

Comment thread Makefile
$(EASK) test ert ./test/*.el

test-activate: package install
$(EASK) emacs --batch -l ./test/activate.el

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might just replace this with ert tests?

@ricardoricho

Copy link
Copy Markdown
Member Author

Hi @jcs090218 I think we are ready to merge/squash this PR.
I moved the ert tests to tests/ert/ directory and now they pass for Emacs
28.2. In the next PR I plan to drop the 27.2 support, add test to the
project-el part of projects and finally add lint to the ci target of the Makefile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants