Le 22/08/2012 09:04, Scott Kostyshak a écrit :
I agree with the above points you made. LFUN_BUFFER_FORALL now supports
multiple views and I tried to add some structure on the terms "visible" and
"hidden", which are now defined in the LFUN documentation. The language is kind
of confusing, but I cannot think of a clearer way. I'm pasting the definitions
here for your thoughts and so that the rest of this email makes sense:

+ * \li Notion: A buffer is `locally visible' with respect to a view if it
+               is visible within that view. If not, it is `locally hidden'.
+               A buffer is `globally visible' if there exists an open view in
+               which it is locally visible. If not, it is `globally hidden'.

Do you really think that 'locally visible' and especially 'locally hidden' have real use cases, or are they just added for the sake of being complete? I would think that all/visible/hidden is good enough.

I decided to use the term "view" in the LFUN documentation. Even though I think
the term "window" is more familiar to users, I think "view" will help the new
user realize the connection of a buffer being "visible" if it is in a "view"
(similar words). Actually, if this is my argument then maybe "viewable" is a
better word than "visible". Any thoughts?

Just use "view (window)" in the description, maybe?

The first argument is now required.

Good.

The first argument is now [lVisible|gVisible|lHidden|gHidden|*]

This is where the extra options have a cost. People have to understand the subtleties of local/global visibility to use the lfun (and the names are ugly :)

The main loop did change a lot. I am a little disappointed that the code had to
get a lot more complicated, but I don't see an easier way and I think it is
worth it. I went through several iterations of different logic and ended up on
the one in the attached patch. I hope I did not fall into the feature-bloat
trap but now most sets of buffers are supported and the terms are well-defined.
I used more comments than usual because of the added complexity. Please let me
know which comments I should remove.

I tested buffer-forall a lot with "Open documents in tabs" checked and 
unchecked.
I'm not sure if this is enough to know how things will work on Mac and Windows.
Should I put out an email asking for testers on lyx-devel?

If you tested correctly these possibilities, I think there is no need to worry. Since the feature does not affect people who do not us it, we have time to act when a bug is found.

Any comments on variable names, style, or general logic would be great.

A few comments:

* in order to make the code more readable, you could move the part that constructs the list of buffers to some new GuiApplication method. It could even be possible to create GuiView helpers if that makes the code easier to understand.

* concerning the complexity, I am not sure why you cannot first get a list ouf buffers to process (the foreach) and than process all of them (the while loop) without making lots of different cases.

* I am not sure it is worth having precise counting of what type of buffers have been processed. Only one message
  "Applied \"%1$s\" to %2$d buffer(s)"
is almost good enough, and showing the list of files names (without paths) instead would be even more informative

There are some things that could still be added. For example, perhaps the user 
wants
to run the LFUN on all buffers that are visible in all windows. "visible in all
windows" (i.e. "locally visible" in all views) is not a supported set by
buffer-forall. Please let me know if you can think of additional functionality
that is missing and that should be added.

I think the biggest risk now is overengineering. My advice is to implement the features that you *know* you need, and wait until people complain that their use case is not covered. From there, you can update the function. For example, it might turn out that there is a need to filter on master/child (think export to PDF).

Trying to cover all possible uses from the start has a cost.

I'm sorry to take up so much of your time on this LFUN, JMarc.

Don't be sorry. Any patch is a good occasion to climb the learning curve of LyX development.

JMarc

PS: I almost forgot. Feel free to disagree with anything I wrote, of course!

Reply via email to