Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extension API for Inline Values #105690

Closed
weinand opened this issue Aug 30, 2020 · 26 comments
Closed

Extension API for Inline Values #105690

weinand opened this issue Aug 30, 2020 · 26 comments
Assignees
Labels
api api-finalization debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan release-notes Release notes issues
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Aug 30, 2020

Today the "Show Inline Values" feature of VS Code's debugger is based on a generic implementation in the VS Code core and provides neither customisability through settings, nor extensibility via extensions.

As a consequence, it is not a perfect fit for all languages (e.g. #101797) and sometimes even shows incorrect values because it doesn't understand the scope regions of the underlying language.

This features asks for an extension API that either replaces the built-in implementation completely or allows to replace parts of the implementation with custom code.

@weinand weinand self-assigned this Aug 30, 2020
@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues api labels Aug 30, 2020
@weinand weinand added this to the On Deck milestone Aug 30, 2020
@weinand
Copy link
Contributor Author

weinand commented Aug 30, 2020

The current generic implementation works as follows:

  • in a debug session VS Code considers only the source of the focused stack frame for showing inline values.
  • VS Code's internal tokenizer is used to find all variables in the source (without understanding scope regions of the language).
  • the potential variables are looked up in the VARIABLE views and for successful matches the value is shown inline in the editor.

This implementation is lightweight and scales well because:

  • instead of a full language parser or AST the implementation piggybacks on the results of the tokenizer.
  • finding the variable values piggybacks on the already fetched variable values available in the VARIABLE view.
  • any false positives of the tokenizer are easily eliminated because there will be no matching variables in the VARIABLES view.

Problems:

@weinand
Copy link
Contributor Author

weinand commented Aug 31, 2020

A first attempt for designing an extension API for inline values tried to use a similar approach as done for the debug hover with the EvaluatableExpressionProvider. In the debug hover case the provider returns an expression for a source location typically determined by the mouse, and then this expression is evaluated by the debug adapter and the result is presented in a hover window.

For the "inline values" feature we tried to reuse the EvaluatableExpressionProvider by introducing a second method that takes a source line number as input and returns an array of {name, expressions} pairs. VS Code would evaluate the expression by the debug adapter and the present the name with the value in the editor at the end of the line.

A prototype based on this approach showed two problem areas:

  • sending multiple "evaluate" requests for every source line to the debug adapter can be slow and affects the stepping performance significantly.
  • Since the language tokenizer is not available in extensions, implementing an EvaluatableExpressionProvider from scratch is difficult. The only realistic way to get this done is by making the EvaluatableExpressionProvider a companion to a language server so that it has access to the parser or AST.

@weinand
Copy link
Contributor Author

weinand commented Jan 12, 2021

During debugging VS Code's "Inline Values" feature shows the value of variables "inline" in the editor:

inline_values

(see https://code.visualstudio.com/updates/v1_9#_inline-variable-values-in-source-code)

Today the "Show Inline Values" feature of VS Code's debugger is based on a single, generic implementation in the VS Code core and provides neither customisability through settings, nor extensibility via extensions.

The proposal is a first attempt to create an extension API for inline values.

Proposal:

The new API is basically a new provider that returns the "Inline values" information (a string) for specific lines of a document.

But there is a twist:
The current "closed" implementation is pretty fast because it relies on the internal tokenizer and looks up values in the Variables view's model. Since extensions do not yet have access to those mechanisms, they are somewhat handicapped today. For that reason, I've introduced the InlineValueContext and InlineValueDescriptor which makes it possible to surface additional information on input (in the InlineValueContext), and additional evaluation strategies when returning values (by returning new types of InlineValueDescriptors).

/**
 * The inline values provider interface defines the contract between extensions and the VS Code inline values feature.
 * In this contract the provider returns inline value information for a given document range
 * and VS Code shows this information in the editor at the end of lines.
 */
export interface InlineValuesProvider {

    /**
     * Provide "inline value" information for a given document and range.
     * VS Code calls this method whenever debugging stops in the given document.
     * The returned inline values information is rendered in the editor at the end of lines.
     *
     * @param document The document for which the inline values information is needed.
     * @param context A bag containing contextual information like the current location.
     * @param token A cancellation token.
     * @return An array of InlineValueDescriptors or a thenable that resolves to such. The lack of a result can be
     * signaled by returning `undefined` or `null`.
     */
    provideInlineValues(document: TextDocument, context: InlineValueContext, token: CancellationToken): ProviderResult<InlineValueDescriptor[]>;
}

/**
 * An open ended information bag passed to the inline value provider.
 * A minimal context containes just the document location where the debugger has stopped.
 * Additional optional information might be scope information or variables and their values.
 */
export interface InlineValueContext {
    /**
     * The document location where execution has stopped.
     * Typically this is used as the end position of the range where inline values are shown.
     */
    position: Position;

    // ... more to come, e.g. Scope information or variable/value candidate information
}

/**
 * Inline value information can exist in multiple forms. The simplest is a direct string value (class InlineValue).
 * A more complex one is an evaluatable expression.
 * The InlineValueDescriptor is just an OR-type of all current and future inline value types.
 */
export type InlineValueDescriptor = InlineValue;

/**
 * An InlineValue represents the value (string) that is shown in the editor at the end of a specific line.
 */
export class InlineValue {
    /**
     * The document's line where to show the inline value.
     */
    readonly line: number;
    /**
     * The value to be shown for the line.
     */
    readonly value: string;
    /**
     * Creates a new InlineValue object.
     *
     * @param line The document line where to show the inline value.
     * @param value The value to be shown for the line.
     */
    constructor(line: number, value: string);
}

export namespace languages {

    /**
     * Register a provider that returns inline values for text documents.
     * In the active debug session VS Code shows this information in the editor at the end of lines.
     *
     * If multiple providers are registered for a language, inline values are concatenated.
     *
     * @param selector A selector that defines the documents this provider is applicable to.
     * @param provider An inline values provider.
     * @return A [disposable](#Disposable) that unregisters this provider when being disposed.
     */
    export function registerInlineValuesProvider(selector: DocumentSelector, provider: InlineValuesProvider): Disposable;
}

@gjsjohnmurray
Copy link
Contributor

InlineValue doc talks about range and expression, yet its properties are named line and value 😕

@weinand
Copy link
Contributor Author

weinand commented Jan 12, 2021

@gjsjohnmurray thanks for spotting this.

@weinand
Copy link
Contributor Author

weinand commented Feb 2, 2021

Here is a new proposal that is better aligned with the "InlineHints" proposal and incorporates feedback from the API sync.

Changes:

  • InlineValuesProvider:
    • provideInlineValues got a new viewPort argument so that a provider can optimise the number of inline values to compute.
    • a onDidChangeInlineValues method was added (similar to the InlineHintsProvider).
  • InlineValueContext:
    • the stoppedLocation property is now a Range instead of a Position.
  • the OR-type InlineValueDescriptor has been renamed to InlineValue and now combines the three types InlineValueText, InlineValueVariableLookup, and InlineValueEvaluatableExpression.
  • the InlineValue type has been renamed to InlineValueText.
    • the line: number attribute has been changed to range: Range with the new semantics that the range denotes the document range of the language construct for which the value is provided (and not the location where the values will be shown).
  • new experimental types InlineValueVariableLookup and InlineValueEvaluatableExpression have been added to illustrate how a provider can leverage VS Code's builtin functionality.
/**
 * The inline values provider interface defines the contract between extensions and the VS Code debugger inline values feature.
 * In this contract the provider returns inline value information for a given document range
 * and VS Code shows this information in the editor at the end of lines.
 */
export interface InlineValuesProvider {

  /**
   * An optional event to signal that inline values have changed.
   * @see [EventEmitter](#EventEmitter)
   */
  onDidChangeInlineValues?: Event<void> | undefined;

  /**
   * Provide "inline value" information for a given document and range.
   * VS Code calls this method whenever debugging stops in the given document.
   * The returned inline values information is rendered in the editor at the end of lines.
   *
   * @param document The document for which the inline values information is needed.
   * @param viewPort The visible document range for which inline values should be computed.
   * @param context A bag containing contextual information like the current location.
   * @param token A cancellation token.
   * @return An array of InlineValueDescriptors or a thenable that resolves to such. The lack of a result can be
   * signaled by returning `undefined` or `null`.
   */
  provideInlineValues(document: TextDocument, viewPort: Range, context: InlineValueContext, token: CancellationToken): ProviderResult<InlineValue[]>;
}

/**
 * An open ended information bag passed to the inline value provider.
 * A minimal context containes just the document location where the debugger has stopped.
 * Additional optional information might be scope information or variables and their values.
 */
export interface InlineValueContext {
  /**
   * The document range where execution has stopped.
   * Typically the end position of the range denotes the line where the inline values are shown.
   */
  stoppedLocation: Range;

  // ... more to come, e.g. Scope information or variable/value candidate information
}

/**
 * Inline value information can be provided by different means:
 * - directly as a text value (class InlineValueText).
 * - as a name to use for a variable lookup (class InlineValueVariableLookup)
 * - as an evaluatable expression (class InlineValueEvaluatableExpression)
 * The InlineValue types combines all inline value types into one type.
 */
export type InlineValue = InlineValueText | InlineValueVariableLookup | InlineValueEvaluatableExpression;

/**
 * Provide inline value as text.
 */
export class InlineValueText {
  /**
   * The text of the inline value.
   */
  readonly text: string;
  /**
   * The range of the inline value.
   */
  readonly range: Range;
  /**
   * Creates a new InlineValueText object.
   *
   * @param text The value to be shown for the line.
   * @param range The document line where to show the inline value.
   */
  constructor(text: string, range: Range);
}

/**
 * Provide inline value through a variable lookup.
 */
export class InlineValueVariableLookup {
  /**
   * The name of the variable to look up.
   */
  readonly variableName: string;
  /**
   * How to perform the lookup.
   */
  readonly caseSensitiveLookup: boolean;
  /**
   * The range of the inline value.
   */
  readonly range: Range;
  /**
   * Creates a new InlineValueVariableLookup object.
   *
   * @param variableName The name of the variable to look up.
   * @param range The document line where to show the inline value.
   * @param caseSensitiveLookup How to perform the lookup. If missing lookup is case sensitive.
   */
  constructor(variableName: string, range: Range, caseSensitiveLookup?: boolean);
}

/**
 * Provide inline value through a expression evaluation.
 */
export class InlineValueEvaluatableExpression {
  /**
   * The expression to evaluate.
   */
  readonly expression: string;
  /**
   * The range of the inline value.
   */
  readonly range: Range;
  /**
   * Creates a new InlineValueEvaluatableExpression object.
   *
   * @param expression The expression to evaluate.
   * @param range The document line where to show the inline value.
   */
  constructor(expression: string, range: Range);
}

export namespace languages {

  /**
   * Register a provider that returns inline values for text documents.
   * If debugging has stopped VS Code shows inline values in the editor at the end of lines.
   *
   * Multiple providers can be registered for a language. In that case providers are asked in
   * parallel and the results are merged. A failing provider (rejected promise or exception) will
   * not cause a failure of the whole operation.
   *
   * @param selector A selector that defines the documents this provider is applicable to.
   * @param provider An inline values provider.
   * @return A [disposable](#Disposable) that unregisters this provider when being disposed.
   */
  export function registerInlineValuesProvider(selector: DocumentSelector, provider: InlineValuesProvider): Disposable;
}

@akaroml
Copy link
Member

akaroml commented Feb 8, 2021

@testforstephen please take a look and provide feedback on behalf of the Java team.

@weinand
Copy link
Contributor Author

weinand commented Feb 17, 2021

A first cut of the proposed API and its implementation has been released.

The proposed API:

//#region inline value provider: https://github.com/microsoft/vscode/issues/105690
/**
* The inline values provider interface defines the contract between extensions and the VS Code debugger inline values feature.
* In this contract the provider returns inline value information for a given document range
* and VS Code shows this information in the editor at the end of lines.
*/
export interface InlineValuesProvider {
/**
* An optional event to signal that inline values have changed.
* @see [EventEmitter](#EventEmitter)
*/
onDidChangeInlineValues?: Event<void> | undefined;
/**
* Provide "inline value" information for a given document and range.
* VS Code calls this method whenever debugging stops in the given document.
* The returned inline values information is rendered in the editor at the end of lines.
*
* @param document The document for which the inline values information is needed.
* @param viewPort The visible document range for which inline values should be computed.
* @param context A bag containing contextual information like the current location.
* @param token A cancellation token.
* @return An array of InlineValueDescriptors or a thenable that resolves to such. The lack of a result can be
* signaled by returning `undefined` or `null`.
*/
provideInlineValues(document: TextDocument, viewPort: Range, context: InlineValueContext, token: CancellationToken): ProviderResult<InlineValue[]>;
}
/**
* An open ended information bag passed to the inline value provider.
* A minimal context containes just the document location where the debugger has stopped.
* Additional optional information might be scope information or variables and their values.
*/
export interface InlineValueContext {
/**
* The document range where execution has stopped.
* Typically the end position of the range denotes the line where the inline values are shown.
*/
stoppedLocation: Range;
// ... more to come, e.g. Scope information or variable/value candidate information
}
/**
* Inline value information can be provided by different means:
* - directly as a text value (class InlineValueText).
* - as a name to use for a variable lookup (class InlineValueVariableLookup)
* - as an evaluatable expression (class InlineValueEvaluatableExpression)
* The InlineValue types combines all inline value types into one type.
*/
export type InlineValue = InlineValueText | InlineValueVariableLookup | InlineValueEvaluatableExpression;
/**
* Provide inline value as text.
*/
export class InlineValueText {
/**
* The text of the inline value.
*/
readonly text: string;
/**
* The range of the inline value.
*/
readonly range: Range;
/**
* Creates a new InlineValueText object.
*
* @param text The value to be shown for the line.
* @param range The document line where to show the inline value.
*/
constructor(text: string, range: Range);
}
/**
* Provide inline value through a variable lookup.
*/
export class InlineValueVariableLookup {
/**
* The name of the variable to look up.
*/
readonly variableName: string;
/**
* How to perform the lookup.
*/
readonly caseSensitiveLookup: boolean;
/**
* The range of the inline value.
*/
readonly range: Range;
/**
* Creates a new InlineValueVariableLookup object.
*
* @param variableName The name of the variable to look up.
* @param range The document line where to show the inline value.
* @param caseSensitiveLookup How to perform the lookup. If missing lookup is case sensitive.
*/
constructor(variableName: string, range: Range, caseSensitiveLookup?: boolean);
}
/**
* Provide inline value through an expression evaluation.
*/
export class InlineValueEvaluatableExpression {
/**
* The expression to evaluate.
*/
readonly expression: string;
/**
* The range of the inline value.
*/
readonly range: Range;
/**
* Creates a new InlineValueEvaluatableExpression object.
*
* @param expression The expression to evaluate.
* @param range The document line where to show the inline value.
*/
constructor(expression: string, range: Range);
}
export namespace languages {
/**
* Register a provider that returns inline values for text documents.
* If debugging has stopped VS Code shows inline values in the editor at the end of lines.
*
* Multiple providers can be registered for a language. In that case providers are asked in
* parallel and the results are merged. A failing provider (rejected promise or exception) will
* not cause a failure of the whole operation.
*
* @param selector A selector that defines the documents this provider is applicable to.
* @param provider An inline values provider.
* @return A [disposable](#Disposable) that unregisters this provider when being disposed.
*/
export function registerInlineValuesProvider(selector: DocumentSelector, provider: InlineValuesProvider): Disposable;
}

Please note:

  • the implementation already supports the inline value types InlineValueText and InlineValueVariableLookup.
  • the inline value type InlineValueEvaluatableExpression does not yet evaluate the expression.

@weinand
Copy link
Contributor Author

weinand commented Feb 19, 2021

I've slightly changed the API to align it with the existing EvaluatableExpressionProvider API (for debug hovers). The first argument for the three InlineValue types is now the document range. In addition the variableName and expression are now optional because they can be automatically extracted from the document based on the range.

With this the InlineValueEvaluatableExpression and the (old) EvaluatableExpression are now identical which raises the question whether we need the InlineValueEvaluatableExpression at all.

Support for InlineValueEvaluatableExpression is now implemented.

@TylerLeonhardt
Copy link
Member

I like your proposal about InlineValueVariableLookup. The PowerShell debug adapter doesn't support the evaluate message as it was intended for but it does implement the variables message as expected.

I see InlineValueVariableLookup relating to the result of the variables message whereas InlineValueEvaluatableExpression relating to the result of the evaluate message.

@weinand did call out that since PowerShell's language and debugger are in the same extension, InlineValueText could just be used to do parsing and value providing in one go... but I still like the associate of the debug messages I called out above.

@weinand
Copy link
Contributor Author

weinand commented Mar 1, 2021

Reopened for further feedback and API finalization

@weinand weinand reopened this Mar 1, 2021
@weinand
Copy link
Contributor Author

weinand commented Mar 2, 2021

Issues for API meeting March 2:

  • I've changed the API of the three InlineValue types to better align with the existing EvaluatableExpressionProvider API (for debug hovers):
    export class EvaluatableExpression {
      // ...
      constructor(range: Range, expression?: string);;
    }
    The first argument for all three InlineValue constructors is now the document range because it is a common and mandatory argument for all InlineValue types. In addition the variableName and expression arguments are now optional because they can be automatically extracted from the underlying document based on the range:
    export class InlineValueText {
      // ...
      constructor(range: Range, text: string);
    }
    
    export class InlineValueVariableLookup {
      // ...
      constructor(range: Range, variableName?: string, caseSensitiveLookup?: boolean);
    }
    
    export class InlineValueEvaluatableExpression {
      // ...
      constructor(range: Range, expression?: string);
    }
  • If we agree that range comes first, we should consider to flip the arguments for the InlineHint constructor too:
    export class InlineHint {
      // ...
      constructor(range: Range, text: string, kind?: InlineHintKind);
    }
  • With the InlineValueEvaluatableExpression and the (old) EvaluatableExpression now being identical raises the question whether we need the InlineValueEvaluatableExpression at all.

Here is the full proposed API.

@weinand
Copy link
Contributor Author

weinand commented Mar 2, 2021

Outcome of the API discussion:

  • making range the first argument is OK and it would make sense for the class InlineHint to follow suit.
  • despite the fact that the proposed class InlineValueEvaluatableExpression is identical to the existing class EvaluatableExpression, we will not remove it from the proposal because we want to have all API features self contained in order to support independent future evolution.

/cc @jrieken

jrieken added a commit that referenced this issue Mar 3, 2021
@testforstephen
Copy link

From the perspective of Java debugger, we need leverage the debug runtime to resolve the duplicated variables from different scopes precisely. Current context just provides the stoppedLocation info, that seems not enough for us to resolve the current stack frame where the execution has stopped, since Java supports multi threads. I will suggest to add the frameId to the context.

Regarding scope, I haven't figured out how the client can make use of it.

export interface InlineValueContext {
  /**
   * The document range where execution has stopped.
   * Typically the end position of the range denotes the line where the inline values are shown.
   */
  stoppedLocation: Range;

 /**
  * The stack frame where the execution has stopped.
  */
  frameId: number;

  // ... more to come, e.g. Scope information or variable/value candidate information
}

cc:// @weinand @akaroml

@weinand
Copy link
Contributor Author

weinand commented Mar 4, 2021

@testforstephen thanks for the feedback.
As suggested I've added a frameId property to the InlineValueContext.

Please try the next Insiders.

@akaroml
Copy link
Member

akaroml commented Mar 8, 2021

I would like to bring up a potential use case of the scope concept as well. I consider this a handy way for users to let the debugger know the info that they really care to see in a debug session.

Currently, the Java debugger only reports one scope to vs code, that is the Local scope, which shows whatever is inside the current frame. But static members are not included by default. Users need to change java.debug.settings.showStaticVariables to true to see those values. We can group them into a new scope, e.g. Global or Static. So that without changing the setting, the users can see those static values by simply expanding the new scope in the VARIABLES view.

The UI state of the scope can also be part of the InlineValueContext as a result. With that, the debugger will only return values that are currently in scope.

@weinand
Copy link
Contributor Author

weinand commented Mar 8, 2021

@akaroml is it really necessary to include the UI-state of the VARIABLE tree's scope-nodes in the InlineValueContext? Since this is a concept that we are not using elsewhere, I'm a bit reluctant...

The extension already has the static information via the java.debug.settings.showStaticVariables setting and it can guess the dynamic expand state based on the last issued scope requests.

@testforstephen
Copy link

The extension already has the static information via the java.debug.settings.showStaticVariables setting and it can guess the dynamic expand state based on the last issued scope requests.

Yes, we're currently using a setting to control the visibility of the static info. But meanwhile, we're evaluating whether to return a static scope directly and collapse it by default using expensive flag. This is more intuitive and easier to spot. So including scope state in the InlineValueContext is helpful for this case.

And below is another feedback about valueFormat.

In the arguments of DAP requests variables and evaluate, there is a property "format?: ValueFormat;". i don't know whether VS Code supports it. If it does, it would be better to add it to InlineValueContext as well because we want to keep the inline value text in the same format as the Variables view.

@akaroml
Copy link
Member

akaroml commented Mar 12, 2021

@weinand please check @testforstephen's comment above. Our goal is to make it easier for users to inspect statics as well. The scope concept will help achieve that in both the regular variable view and the inline value view.

@weinand
Copy link
Contributor Author

weinand commented Mar 16, 2021

@testforstephen, @akaroml thanks for your input!

here are my comments:

  • VARIABLE View's Scope UI state: I understand that including the expand state of the scope nodes in the VARIABLEs view would be helpful in your specific case. But I'm not yet convinced that we should bake this UI state into the API this early. Such a concept is not used in VS Code's extension API anywhere else because we try to avoid coupling an extension API too closely with a specific UI implementation. For the initial version of the Inline Values API I suggest that you determine the expand state of the scope nodes yourself by tracking the DAP's scope requests. If there was a scope request for your "static" scope, then you know that the "static" scope node has been expanded and that the Inline Values provider should return static values as well.
  • "format": "ValueFormat": VS Code does not support/use the format argument on the variables and evaluate requests. So, it does not really make sense to introduce a property on the InlineValueContext for which we do not have all the details. But as soon as VS Code will start using it, we will add "format" as an optional property on InlineValueContext.

@weinand
Copy link
Contributor Author

weinand commented Mar 16, 2021

Proposed API for the Inline Values Provider.

Since the last API discussion a frameId attribute has been added to the open-ended InlineValueContext object.

/**
 * A value-object that contains contextual information when requesting inline values from a InlineValuesProvider.
 */
export interface InlineValueContext {

  /**
   * The stack frame (as a DAP Id) where the execution has stopped.
   */
  readonly frameId: number;

  /**
   * The document range where execution has stopped.
   * Typically the end position of the range denotes the line where the inline values are shown.
   */
  readonly stoppedLocation: Range;
}

@weinand
Copy link
Contributor Author

weinand commented Mar 16, 2021

The InlineValuesProvider API has been finalized and will appear in the next Insiders.

@weinand
Copy link
Contributor Author

weinand commented Mar 22, 2021

In this milestone we've finalised the API without changing it. So the test-plan item from the previous milestone still covers it.

@testforstephen
Copy link

The API looks good. Here are some feedbacks about the implementation.

  1. We want to use the viewport to crop the results in cases where there is a large list of variables. But the viewPort parameter from provideInlineValues API never changes when i scroll the editor.
    For example, in my test case the initial viewport range is L80 ~ L126, and it doesn't change when i scroll up.
Screen.Recording.2021-03-23.at.15.34.55.mov
  1. When Java debugger uses InlineValuesProvider to provide inline debugging feature, the user still needs to manually enable the user setting "debug.inlineValues" to get the feature. Is this by design?

  2. When the user uses the "Set Value" feature to update the variables, the inline values are not refreshed.

image

@weinand
Copy link
Contributor Author

weinand commented Mar 23, 2021

@testforstephen thanks for the feedback.

I've created issues #119559, #119560, #119564 for it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api api-finalization debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan release-notes Release notes issues
Projects
None yet
Development

No branches or pull requests

7 participants
@weinand @TylerLeonhardt @gjsjohnmurray @testforstephen @akaroml and others