Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hello, Adam Porter writes: > I looked at that file and I see that it's pretty basic. There's a first step in everything. > I recently put together a sort of testing framework for > org-super-agenda that makes it pretty easy to develop and run > consistent tests on agendas: > > https://github.c

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

By the way, one of its features is that, when a test doesn't pass, it shows a line-by-line diff of the agenda buffer compared with the saved result. It's saved me a lot of time when debugging test failures, as otherwise I'd have to visually compare a large agenda buffer. :)

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Nicolas Goaziou writes: > If you feel like it, some tests in "test-org-agenda.el" would be nice, > too. Hi Nicolas, I looked at that file and I see that it's pretty basic. I recently put together a sort of testing framework for org-super-agenda that makes it pretty easy to develop and run cons

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hello, Adam Porter writes: > Here is another patch series with the requested changes. Applied. Thank you! If you feel like it, some tests in "test-org-agenda.el" would be nice, too. Regards, -- Nicolas Goaziou

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hi Nicolas, Here is another patch series with the requested changes. Thanks, Adam >From 98a90c8e05acc8733972be731e5a9eb1efe10fee Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 19 Aug 2017 21:26:12 -0500 Subject: [PATCH 1/2] org-agenda: Refactor org-agenda-overriding-header code * lisp/or

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hello, Adam Porter writes: > You're right, that's why IIRC I used cl-defmacro originally. The > issue here seems to be using the keyword argument. It seemed like a > good idea to specify it with the ":default: keyword argument for two > reasons: > > 1. To make it explicitly clear that the bod

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hi Nicolas, Nicolas Goaziou writes: > Adam Porter writes: > >> Here are the patches. Please let me know if any other changes are >> needed. > > Thank you! Comments follow. > >> +(defmacro org-agenda--insert-overriding-header (&key default) > > There is no "&key" in `defmacro'. You're right, t

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Adam Porter writes: > Here are the patches. Please let me know if any other changes are > needed. Thank you! Comments follow. > +(defmacro org-agenda--insert-overriding-header (&key default) There is no "&key" in `defmacro'. It should be (default). > + "Insert header into agenda view depen

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hello, Adam Porter writes: > If you would prefer the &rest approach, that would mean doing the actual > insertion into the agenda buffer in the macro call rather than the macro > expansion. I thought it would be better to separate the actual > insertion by abstracting it behind the macro. I do

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Adam Porter writes: > I'll post a new patch soon. Thanks. Hi Nicolas, Here are the patches. Please let me know if any other changes are needed. Thanks, Adam >From 2b938e98e2cc2409044eb7b03bff98b0a37d404e Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 19 Aug 2017 21:26:12 -0500 Subje

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hi Nicolas, Nicolas Goaziou writes: > Nitpick: the paragraph above usually comes after the list of changes. Fixed. > Nitpick: you can only write "Use macro" once, on the last line. Fixed. > Why `cl-defmacro'? Usually, `defmacro' is enough. Fixed. > While we're at it, > > `org-add-props'

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hello, Adam Porter writes: > Here is the new patch. Thank you. > I took the liberty of using a macro to replace the code that was > duplicated in the four agenda functions. Please let me know if you > would like any further changes. Some comments follow. > From 203bc583da0c482ab7092623383fe4

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Nicolas Goaziou writes: > Let's adjust the patch, then. Thank you! Hi Nicolas, Here is the new patch. I took the liberty of using a macro to replace the code that was duplicated in the four agenda functions. Please let me know if you would like any further changes. Thanks, Adam>From 203bc583

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hello, Adam Porter writes: > I almost did that, making it not insert a header if it was set to an > empty string, but I thought that this approach gives more flexibility, > allowing users to insert a blank header if they want. "\n" would allow to insert a blank header. > But if you would prefe

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Nicolas Goaziou writes: > What about "fixing" the empty string case, i.e., not inserting a blank > line when set to the empty string? Hi Nicolas, I almost did that, making it not insert a header if it was set to an empty string, but I thought that this approach gives more flexibility, allowing

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hello, Adam Porter writes: > This patch adds a 'none setting for org-agenda-overriding-header, which > allows it to be disabled completely, rather than inserting a blank line > as it does when set to an empty string. Thank you. What about "fixing" the empty string case, i.e., not inserting a b

[O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header

Hi, This patch adds a 'none setting for org-agenda-overriding-header, which allows it to be disabled completely, rather than inserting a blank line as it does when set to an empty string. This was requested by a user here: , but I guess it