disparity icon indicating copy to clipboard operation
disparity copied to clipboard

Support passing additional options instead of requiring export mutation

Open G-Rath opened this issue 4 years ago • 3 comments

Currently to change things like added and removed, you have to mutate an export, which means you're changing it globally.

It'd be great if the opts object supported taking these exports as properties:

export interface Options {
  /**
   * How many lines to display before/after a line that contains diffs
   *
   * @default 3
   */
  context?: number;
  /**
   * File paths displayed just before the diff
   *
   * @default [disparity.removed, disparity.added]
   */
  paths?: [string, string];
  /**
   * The string used on diff headers to say that chars/lines were removed
   *
   * @default 'removed'
   */
  removed?: string;
  /**
   * The string used on the diff headers to say that chars/lines were added
   *
   * @default 'added'
   */
  added?: string;
  /**
   * Object containing references to all the colors used by disparity.
   */
  colors?: {
    added?: DiffColor;
    charsAdded?: DiffColor;
    charsRemoved?: DiffColor;
    header?: DiffColor;
    removed?: DiffColor;
    section?: DiffColor;
  };
}

I'm happy to make a PR implement this if it'd be welcomed :)

G-Rath avatar Feb 16 '20 10:02 G-Rath

yeah def agree, it would make for a much better api 👍

one thing though is that given how long the library has been around I think it would make a lot of sense to preserve the old API and have the options live alongside with the exports (I'd really prefer to avoid forcing breaking changes downstream) - so have it in a way that the options take precedence but defaults to using the old exports definitions if undefined

What do you think? 😊

ruyadorno avatar May 19 '20 02:05 ruyadorno

I think that's an ok way to go.

Then you can just drop the support as part of the next major (if/when that happens) if you wanted too.

G-Rath avatar May 19 '20 03:05 G-Rath

sounds good 👍

ruyadorno avatar May 19 '20 17:05 ruyadorno