Thanks for the thoughts. I like the idea of handling behavior modification through the FormulaEvaluator, as those can be discarded and recreated, and are tied to a specific workbook. I don't like ThreadLocal options, even though POI is not at all Thread-safe, as it is always harder to debug and maintain. Especially if one thread is handling multiple workbooks, it could be confusing to see side effects.
Fresh eyes this morning show me that Function.evaluate() is only called from 3 places - OperationEvaluationFactory, and two implementations of Function that need a variation on recursion (SUBTOTAL and conditional formatting ABOVE_AVERAGE). Further, OperationEvaluationFactory calls it from a static method that has OperationEvaluationContext as a parameter. That context class already has a flag for single vs. multiple results. Perhaps that's the right place to store additional flags, like one for identifying which return value to use for HYPERLINK. The interface for Function would change to add OperationEvaluationContext as a parameter to evaluate(). The recursive calls would then have this value available to pass to their child function calls, and only the instantiation path for the standard factory call would need adjusting to handle evaluation flags of any sort. The interface could then have a default method with the current signature, and call the updated method with a null context. Only functions that actually need or care about flags would be affected then, and could be coded to handle a null context by invoking the existing behavior, only adjusting evaluation in the presence of their specific flag in a non-null context. Work remains to figure out what calling path makes the most sense for actually setting a flag in the context, but I would anticipate only implementing it in one path, perhaps as you suggest through the FormulaEvaluator via instance specific getter/setter methods for hard-coded or parameterized flags. On Thu, Nov 29, 2018 at 11:51 PM Nick Burch <apa...@gagravarr.org> wrote: > On Thu, 29 Nov 2018, Greg Woolsey wrote: > > The first argument to the function is the target URL, the second argument > > is optional display text. > > > > The current implementation returns the display text, if present, as the > > function value. This is correct, as that's the value displayed, but if > you > > want to actually implement a link, one needs the URL argument also. > > I'd lean towards one of: > * ThreadLocal that the hyperlink function checks (with helpful setter and > clearer on it) to toggle between returning the text or link > * setter on FormulaEvaluator that lets you toggle, then pass that > somehow to the hyperlink function > * subclass of FormulaEvaluator with extra evaluateCell method that > returns the link, plus similar to above way to pass flag to function > > Nick > > --------------------------------------------------------------------- > To unsubscribe, e-mail: user-unsubscr...@poi.apache.org > For additional commands, e-mail: user-h...@poi.apache.org > >
