message: force rebuild on isOnboarding and uncommitted changes

diff:
@@ -8,9 +8,22 @@ const CreateAppTokenMutation = `
createAppToken(code: $projectToken)
}
`;
+
+const AppByCodeQuery = `
+ query AppyByCodeQuery($code: String!) {
+ appByCode(code: $code) {
+ isOnboarding
+ }
+ }
+`;
interface CreateAppTokenMutationResult {
createAppToken: string;
}
+interface AppyByCodeQueryResult {
+ appByCode: {
+ isOnboarding: boolean;
+ };
+}
export const setAuthorizationToken = async (ctx: Context) => {
const { client, options } = ctx;
@@ -22,6 +35,10 @@ export const setAuthorizationToken = async (ctx: Context) => {
variables
);
client.setAuthorization(appToken);
+ const { appByCode: app } = await ctx.client.runQuery<AppyByCodeQueryResult>(AppByCodeQuery, {
+ code: ctx.options.projectToken,
+ });
+ ctx.isOnboarding = app.isOnboarding;
} catch (errors) {
if (errors[0] && errors[0].message && errors[0].message.match('No app with code')) {
throw new Error(invalidProjectToken(variables));

diff:
@@ -164,6 +164,10 @@ export const setGitInfo = async (ctx: Context, task: Task) => {
ctx.log.debug(e);
}
+ if (ctx.isOnboarding && ctx.git.changedFiles && ctx.git.changedFiles.length) {
+ ctx.options.forceRebuild = true;
+ }
+
if (ctx.options.externals && ctx.git.changedFiles && ctx.git.changedFiles.length) {
// eslint-disable-next-line no-restricted-syntax
for (const glob of ctx.options.externals) {

diff:
@@ -176,7 +176,6 @@ export const createBuild = async (ctx: Context, task: Task) => {
ctx.build = build;
ctx.isPublishOnly = !build.features.uiReview && !build.features.uiTests;
- ctx.isOnboarding = build.number === 1 || (build.autoAcceptChanges && !autoAcceptChanges);
if (list) {
ctx.log.info(listingStories(build.tests));


message: remove unneccessary try/catch

change_type: MODIFY

diff:
@@ -81,6 +81,7 @@ jest.mock('node-fetch', () =>
billingUrl: 'https://foo.bar',
exceededThreshold: false,
paymentRequired: false,
+ setupUrl: 'https://setup.com',
},
},
tests: [
@@ -140,6 +141,16 @@ jest.mock('node-fetch', () =>
};
}
+ if (query.match('AppyByCodeQuery')) {
+ return {
+ data: {
+ appByCode: {
+ isOnboarding: true,
+ },
+ },
+ };
+ }
+
throw new Error(`Unknown Query: ${query}`);
},
}))

diff:
-import { setAuthorizationToken } from './auth';
+import { getAppInfo, setAuthorizationToken } from './auth';
describe('setAuthorizationToken', () => {
it('updates the GraphQL client with an app token from the index', async () => {
@@ -9,3 +9,19 @@ describe('setAuthorizationToken', () => {
expect(client.setAuthorization).toHaveBeenCalledWith('token');
});
});
+
+describe('getAppInfo', () => {
+ it('retreives onboarding info using project token', async () => {
+ const client = { runQuery: jest.fn(), setAuthorization: jest.fn() };
+ client.runQuery.mockReturnValue({ appByCode: { isOnboarding: true } });
+
+ const ctx = {
+ client,
+ options: { projectToken: 'test' },
+ isOnboarding: false,
+ };
+
+ await getAppInfo(ctx as any);
+ expect(ctx.isOnboarding).toBeTruthy();
+ });
+});

diff:
@@ -9,9 +9,9 @@ const CreateAppTokenMutation = `
}
`;
-const AppByCodeQuery = `
- query AppyByCodeQuery($code: String!) {
- appByCode(code: $code) {
+const AppQuery = `
+ query AppQuery {
+ app {
isOnboarding
}
}
@@ -19,8 +19,8 @@ const AppByCodeQuery = `
interface CreateAppTokenMutationResult {
createAppToken: string;
}
-interface AppyByCodeQueryResult {
- appByCode: {
+interface AppQueryResult {
+ app: {
isOnboarding: boolean;
};
}
@@ -35,10 +35,6 @@ export const setAuthorizationToken = async (ctx: Context) => {
variables
);
client.setAuthorization(appToken);
- const { appByCode: app } = await ctx.client.runQuery<AppyByCodeQueryResult>(AppByCodeQuery, {
- code: ctx.options.projectToken,
- });
- ctx.isOnboarding = app.isOnboarding;
} catch (errors) {
if (errors[0] && errors[0].message && errors[0].message.match('No app with code')) {
throw new Error(invalidProjectToken(variables));
@@ -47,7 +43,20 @@ export const setAuthorizationToken = async (ctx: Context) => {
}
};
+export const getAppInfo = async (ctx: Context) => {
+ const { client, options } = ctx;
+ const variables = { code: options.projectToken };
+
+ const { app } = await client.runQuery<AppQueryResult>(AppQuery, variables);
+ ctx.isOnboarding = app.isOnboarding;
+};
+
export default createTask({
title: initial.title,
- steps: [transitionTo(authenticating), setAuthorizationToken, transitionTo(authenticated, true)],
+ steps: [
+ transitionTo(authenticating),
+ setAuthorizationToken,
+ getAppInfo,
+ transitionTo(authenticated, true),
+ ],
});

diff:
@@ -94,6 +94,9 @@ export const setGitInfo = async (ctx: Context, task: Task) => {
ctx.git.parentCommits = parentCommits;
ctx.log.debug(`Found parentCommits: ${parentCommits.join(', ')}`);
+ if (ctx.isOnboarding) {
+ ctx.options.forceRebuild = true;
+ }
// If we're running against the same commit as the sole parent, then this is likely a rebuild (rerun of CI job).
// If the MRA is all green, there's no need to rerun the build, we just want the CLI to exit 0 so the CI job succeeds.
// This is especially relevant for (unlinked) projects that don't use --exit-zero-on-changes.
@@ -164,10 +167,6 @@ export const setGitInfo = async (ctx: Context, task: Task) => {
ctx.log.debug(e);
}
- if (ctx.isOnboarding && ctx.git.changedFiles && ctx.git.changedFiles.length) {
- ctx.options.forceRebuild = true;
- }
-
if (ctx.options.externals && ctx.git.changedFiles && ctx.git.changedFiles.length) {
// eslint-disable-next-line no-restricted-syntax
for (const glob of ctx.options.externals) {

diff:
@@ -136,7 +136,7 @@ export interface Context {
stopApp?: () => void;
closeTunnel?: () => void;
isPublishOnly?: boolean;
- isOnboarding?: boolean;
+ isOnboarding: boolean;
http: {
fetch: (url: string, options?: RequestInit, opts?: any) => Promise<Response>;

diff:
@@ -4,12 +4,20 @@ export default {
title: 'CLI/Messages/Errors',
};
-export const BuildHasChanges = () =>
- buildHasChanges({
+const context = {
build: {
number: 42,
changeCount: 2,
webUrl: 'https://www.chromatic.com/build?appId=59c59bd0183bd100364e1d57&number=42',
+ app: {
+ setupUrl: 'https://www.chromatic.com/setup?appId=59c59bd0183bd100364e1d57',
+ },
},
exitCode: 1,
- });
+ isOnboarding: false,
+};
+
+export const BuildHasChangesNotOnboarding = () => buildHasChanges(context);
+
+export const BuildHasChangesIsOnboarding = () =>
+ buildHasChanges({ ...context, isOnboarding: true });

diff:
@@ -5,10 +5,12 @@ import { dedent } from 'ts-dedent';
import { error, info } from '../../components/icons';
import link from '../../components/link';
-export default ({ build, exitCode }) => {
+export default ({ build, exitCode, isOnboarding }) => {
const changes = pluralize('visual changes', build.changeCount, true);
return dedent(chalk`
- ${error} {bold Found ${changes}}: Review the changes at ${link(build.webUrl)}
+ ${error} {bold Found ${changes}}: Review the changes at ${link(
+ isOnboarding ? build.app.setupUrl : build.webUrl
+ )}
${info} For CI/CD use cases, this command failed with exit code ${exitCode}
Pass {bold --exit-zero-on-changes} to succeed this command regardless of changes.

diff:
import task from '../components/task';
-import { BuildHasChanges } from '../messages/errors/buildHasChanges.stories';
+import { BuildHasChangesNotOnboarding } from '../messages/errors/buildHasChanges.stories';
import { BuildPassed, FirstBuildPassed } from '../messages/info/buildPassed.stories';
import { Intro } from '../messages/info/intro.stories';
import { StorybookPublished } from '../messages/info/storybookPublished.stories';
@@ -147,7 +147,7 @@ export const ChangesFound = () => [
verify.Published,
snapshot.BuildComplete
),
- BuildHasChanges(),
+ BuildHasChangesNotOnboarding(),
];
export const FirstBuild = () => [

license: MIT License

repo: chromaui/chromatic-cli

original_message: remove unneccessary try/catch


message: add test for forceRebuild

diff:
@@ -38,6 +38,7 @@ const commitInfo = {
fromCI: false,
ciService: undefined,
};
+const client = { runQuery: jest.fn(), setAuthorization: jest.fn() };
beforeEach(() => {
getCommitAndBranch.mockResolvedValue(commitInfo);
@@ -46,11 +47,12 @@ beforeEach(() => {
getChangedFilesWithReplacement.mockResolvedValue({ changedFiles: [] });
getVersion.mockResolvedValue('Git v1.0.0');
getSlug.mockResolvedValue('user/repo');
+ client.runQuery.mockReturnValue({ app: { isOnboarding: false } });
});
describe('setGitInfo', () => {
it('sets the git info on context', async () => {
- const ctx = { log, options: {} } as any;
+ const ctx = { log, options: {}, client } as any;
await setGitInfo(ctx, {} as any);
expect(ctx.git).toMatchObject({
commit: '123asdf',
@@ -62,7 +64,7 @@ describe('setGitInfo', () => {
});
it('supports overriding the owner name in the slug', async () => {
- const ctx = { log, options: { ownerName: 'org' } } as any;
+ const ctx = { log, options: { ownerName: 'org' }, client } as any;
await setGitInfo(ctx, {} as any);
expect(ctx.git).toMatchObject({ slug: 'org/repo' });
});
@@ -72,7 +74,7 @@ describe('setGitInfo', () => {
getChangedFilesWithReplacement.mockResolvedValue({
changedFiles: ['styles/main.scss', 'lib/utils.js'],
});
- const ctx = { log, options: { onlyChanged: true } } as any;
+ const ctx = { log, options: { onlyChanged: true }, client } as any;
await setGitInfo(ctx, {} as any);
expect(ctx.git.changedFiles).toEqual(['styles/main.scss', 'lib/utils.js']);
expect(ctx.git.replacementBuildIds).toEqual([]);
@@ -88,7 +90,7 @@ describe('setGitInfo', () => {
commit: '987bca',
},
});
- const ctx = { log, options: { onlyChanged: true } } as any;
+ const ctx = { log, options: { onlyChanged: true }, client } as any;
await setGitInfo(ctx, {} as any);
expect(ctx.git.changedFiles).toEqual(['styles/main.scss', 'lib/utils.js']);
expect(ctx.git.replacementBuildIds).toEqual([['rebased', 'parent']]);
@@ -99,8 +101,15 @@ describe('setGitInfo', () => {
getChangedFilesWithReplacement.mockResolvedValue({
changedFiles: ['styles/main.scss', 'lib/utils.js'],
});
- const ctx = { log, options: { onlyChanged: true, externals: ['**/*.scss'] } } as any;
+ const ctx = { log, options: { onlyChanged: true, externals: ['**/*.scss'] }, client } as any;
await setGitInfo(ctx, {} as any);
expect(ctx.git.changedFiles).toBeNull();
});
+
+ it('forces rebuild automatically if app is onboarding', async () => {
+ client.runQuery.mockReturnValue({ app: { isOnboarding: true } });
+ const ctx = { log, options: { ownerName: 'org' }, client } as any;
+ await setGitInfo(ctx, {} as any);
+ expect(ctx.options.forceRebuild).toBe(true);
+ });
});

original_message: add test for forceRebuild


message: add storybook config fix

diff:
@@ -6,6 +6,9 @@ export default function getStorybookConfiguration(
shortName: string,
longName?: string
) {
+ if (!storybookScript) {
+ return null;
+ }
const parts = storybookScript.split(/[\s='"]+/);
let index = parts.indexOf(longName);
if (index === -1) {

diff:
@@ -4,4 +4,4 @@ export default {
title: 'CLI/Components/Link',
};
-export const Default = () => link('https://www.chromatic.com');
+export const Default = () => link('https://www.chromatic.org');

original_message: add storybook config fix


message: Filter out unsupported addons from sb metadata

change_type: ADD

diff:
+{
+ "generatedAt": 1654054690474,
+ "builder": {
+ "name": "webpack5"
+ },
+ "hasCustomBabel": false,
+ "hasCustomWebpack": true,
+ "hasStaticDirs": false,
+ "hasStorybookEslint": false,
+ "refCount": 0,
+ "packageManager": {
+ "type": "yarn",
+ "version": "1.22.18"
+ },
+ "features": {
+ "postcss": false
+ },
+ "storybookVersion": "6.5.6",
+ "language": "typescript",
+ "storybookPackages": {
+ "@storybook/addon-essentials": {
+ "version": "6.5.6"
+ },
+ "@storybook/builder-webpack5": {
+ "version": "6.5.6"
+ },
+ "@storybook/eslint-config-storybook": {
+ "version": "3.1.2"
+ },
+ "@storybook/linter-config": {
+ "version": "3.1.2"
+ },
+ "@storybook/manager-webpack5": {
+ "version": "6.5.6"
+ },
+ "@storybook/react": {
+ "version": "6.5.6"
+ }
+ },
+ "framework": {
+ "name": "react"
+ },
+ "addons": {
+ "@storybook/addon-viewport": {
+ "version": "6.5.6"
+ },
+ "storybook-addon-apollo-client": {
+ "version": "4.0.11"
+ }
+ }
+}

diff:
@@ -51,10 +51,12 @@ export const getStorybookMetadataFromProjectJson = async (
viewLayer: sbProjectJson.framework.name ?? null,
version: sbProjectJson.storybookPackages[viewLayerPackage].version ?? null,
builder,
- addons: Object.entries(sbProjectJson.addons).map(([packageName, addon]) => ({
+ addons: Object.entries(sbProjectJson.addons)
+ .map(([packageName, addon]) => ({
name: supportedAddons[packageName],
packageName,
packageVersion: addon.version,
- })),
+ }))
+ .filter((addon) => addon.name),
};
};

diff:
@@ -157,5 +157,24 @@ describe('getStorybookInfo', () => {
builder: null,
});
});
+
+ it('does not return unsupported addons in metadata', async () => {
+ const ctx = getContext({
+ options: { storybookBuildDir: 'bin-src/__mocks__/unsupportedAddons' },
+ packageJson: { dependencies: REACT },
+ });
+ await expect(getStorybookInfo(ctx)).resolves.toEqual({
+ addons: [
+ {
+ name: 'viewport',
+ packageName: '@storybook/addon-viewport',
+ packageVersion: '6.5.6',
+ },
+ ],
+ builder: { name: 'webpack5', packageVersion: '6.5.6' },
+ version: '6.5.6',
+ viewLayer: 'react',
+ });
+ });
});
});


message: revert link change

diff:
@@ -4,4 +4,4 @@ export default {
title: 'CLI/Components/Link',
};
-export const Default = () => link('https://www.chromatic.org');
+export const Default = () => link('https://www.chromatic.com');

original_message: revert link change


message: move filtering up in order

diff:
@@ -52,11 +52,11 @@ export const getStorybookMetadataFromProjectJson = async (
version: sbProjectJson.storybookPackages[viewLayerPackage].version ?? null,
builder,
addons: Object.entries(sbProjectJson.addons)
+ .filter(([packageName]) => supportedAddons[packageName])
.map(([packageName, addon]) => ({
name: supportedAddons[packageName],
packageName,
packageVersion: addon.version,
- }))
- .filter((addon) => addon.name),
+ })),
};
};

original_message: move filtering up in order


message: Add change count to buildPassed messages if changes exist.

diff:
import chalk from 'chalk';
import { dedent } from 'ts-dedent';
+import pluralize from 'pluralize';
import { Context } from '../../../types';
import { info, success } from '../../components/icons';
@@ -8,6 +9,7 @@ import { stats } from '../../tasks/snapshot';
export default ({ build, isOnboarding }: Context) => {
const { changes, snapshots, components, stories } = stats({ build });
+ const visualChanges = pluralize('visual changes', build.changeCount, true);
if (isOnboarding) {
return dedent(chalk`
${success} {bold Build passed. Welcome to Chromatic!}
@@ -23,7 +25,7 @@ export default ({ build, isOnboarding }: Context) => {
`)
: dedent(chalk`
${success} {bold Build ${build.number} passed!}
- No visual changes were found in this build.
+ ${build.changeCount === 0 ? 'No visual changes' : visualChanges} were found in this build.
${info} View build details at ${link(build.webUrl)}
`);
};


message: Update logic check

diff:
@@ -9,6 +9,16 @@ export const BuildPassed = () =>
build: {
number: 42,
webUrl: 'https://www.chromatic.com/build?appId=59c59bd0183bd100364e1d57&number=42',
+ changeCount: 0,
+ },
+ } as any);
+
+export const BuildPassedWithChanges = () =>
+ buildPassed({
+ build: {
+ number: 42,
+ webUrl: 'https://www.chromatic.com/build?appId=59c59bd0183bd100364e1d57&number=42',
+ changeCount: 2,
},
} as any);

diff:
@@ -25,7 +25,7 @@ export default ({ build, isOnboarding }: Context) => {
`)
: dedent(chalk`
${success} {bold Build ${build.number} passed!}
- ${build.changeCount === 0 ? 'No visual changes' : visualChanges} were found in this build.
+ ${build.changeCount > 0 ? visualChanges : 'No visual changes'} were found in this build.
${info} View build details at ${link(build.webUrl)}
`);
};

original_message: Update logic check


message: Removing webpack stats flag from build storybook

diff:
],
"scripts": {
"build": "npm-run-all --serial -l bundle:**",
- "build-storybook": "build-storybook -s static --webpack-stats-json",
+ "build-storybook": "build-storybook -s static",
"build-test-storybook": "cross-env SMOKE_TEST=true build-storybook -o test-storybook",
"build-subdir": "cd subdir ; yarn build ; cd ..",
"chromatic": "./bin/main.cjs",


message: Remove script and update git ignore

old_path: .gitignore

new_path: .gitignore

diff:
@@ -3,6 +3,7 @@ node_modules
.env
*.log
*.tgz
+.editorconfig
action/main.js
storybook-static

diff:
"trim-stats": "node -r esm bin-src/trim-stats-file.js",
"storybook": "start-storybook -p 9009 -s static",
"test": "jest",
- "test:file": "jest -t",
"prepare": "husky install",
"bundle:bin": "webpack --config=webpack-bin.config.js && chmod +x ./bin/main.cjs",
"bundle:action": "webpack --config=webpack-action.config.js",

original_message: Remove script and update git ignore


message: Added a more elaborate example for untraced

diff:
@@ -438,6 +438,79 @@ describe('getDependentStoryFiles', () => {
expect.stringContaining(chalk`Found a static file change in {bold path/to/statics/image.png}`)
);
});
+ it('ignores untraced files and dependencies', async () => {
+ const changedFiles = ['src/stories/Button.jsx', 'src/stories/Page.jsx', 'build-storybook.log'];
+ const modules = [
+ {
+ id: './src/stories/Button.jsx', // changed
+ name: './src/stories/Button.jsx + 1 modules',
+ reasons: [
+ { moduleName: './src/stories/Button.stories.jsx' },
+ { moduleName: './src/stories/Header.jsx + 1 modules' },
+ ],
+ },
+ {
+ id: './src/stories/Header.jsx',
+ name: './src/stories/Header.jsx + 1 modules',
+ reasons: [
+ { moduleName: './src/stories/Header.stories.jsx' },
+ { moduleName: './src/stories/Page.stories.jsx + 2 modules' },
+ { moduleName: './src/stories/Page.jsx' },
+ ],
+ },
+ {
+ id: null,
+ name: './src/stories/Page.jsx', // changed
+ reasons: [{ moduleName: './src/stories/Page.stories.jsx' }],
+ },
+ {
+ id: null,
+ name: './src/stories/button.css',
+ reasons: [{ moduleName: './src/stories/Button.jsx' }],
+ },
+ {
+ id: null,
+ name: './src/stories/header.css',
+ reasons: [{ moduleName: './src/stories/Header.jsx' }],
+ },
+ {
+ id: null,
+ name: './src/stories/page.css',
+ reasons: [{ moduleName: './src/stories/Page.jsx' }],
+ },
+ {
+ id: './src/stories/Button.stories.jsx',
+ name: './src/stories/Button.stories.jsx',
+ reasons: [{ moduleName: CSF_GLOB }],
+ },
+ {
+ id: './src/stories/Header.stories.jsx',
+ name: './src/stories/Header.stories.jsx',
+ reasons: [{ moduleName: CSF_GLOB }],
+ },
+ {
+ id: './src/stories/Page.stories.jsx',
+ name: './src/stories/Page.stories.jsx + 2 modules',
+ modules: [
+ { name: './src/stories/Page.stories.jsx' },
+ { name: './src/stories/Page.jsx' },
+ { name: './src/stories/page.css' },
+ ],
+ reasons: [{ moduleName: CSF_GLOB }],
+ },
+ {
+ id: 999,
+ name: CSF_GLOB,
+ reasons: [{ moduleName: './.storybook/generated-stories-entry.js' }],
+ },
+ ];
+ const ctx = getContext({
+ staticDir: ['public'],
+ untraced: ['**/stories/**'],
+ });
+ const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
+ expect(res).toEqual({});
+ });
it('ignores untraced files', async () => {
const changedFiles = ['src/utils.js'];


message: Update stats process to use stream

diff:
@@ -29,6 +29,7 @@ import {
} from '../ui/tasks/upload';
import { Context, Task } from '../types';
import { exitCodes, setExitCode } from '../lib/setExitCode';
+import { readStatsFile } from '../trim-stats-file';
const GetUploadUrlsMutation = `
mutation GetUploadUrlsMutation($paths: [String!]!) {
@@ -154,7 +155,7 @@ export const traceChangedFiles = async (ctx: Context, task: Task) => {
const statsPath = join(ctx.sourceDir, ctx.fileInfo.statsPath);
const { changedFiles } = ctx.git;
try {
- const stats = await fs.readJson(statsPath);
+ const stats = await readStatsFile(statsPath);
const onlyStoryFiles = await getDependentStoryFiles(ctx, stats, statsPath, changedFiles);
if (onlyStoryFiles) {
ctx.onlyStoryFiles = onlyStoryFiles;

diff:
-import fs from 'fs-extra';
import meow from 'meow';
import { getDependentStoryFiles } from './lib/getDependentStoryFiles';
import { Context } from './types';
+import { readStatsFile } from './trim-stats-file';
/**
* Utility to trace a set of changed file paths to dependent story files using a Webpack stats file.
@@ -85,7 +85,7 @@ export async function main(argv: string[]) {
traceChanged: flags.mode || true,
},
} as any;
- const stats = await fs.readJson(flags.statsFile);
+ const stats = await readStatsFile(flags.statsFile);
const changedFiles = input.map((f) => f.replace(/^\.\//, ''));
await getDependentStoryFiles(ctx, stats, flags.statsFile, changedFiles);

diff:
import { parseChunked } from '@discoveryjs/json-ext';
-import { createReadStream, outputFile } from 'fs-extra';
+import { createReadStream, outputFile, existsSync } from 'fs-extra';
import { Stats } from './types';
const dedupe = <T>(arr: T[]) => Array.from(new Set(arr));
@@ -8,8 +8,12 @@ const isUserCode = ({ name, moduleName = name }: { name?: string; moduleName?: s
!moduleName.startsWith('(webpack)') &&
!moduleName.match(/(node_modules|webpack\/runtime)\//);
-export const readStatsFile = async (filePath: string): Promise<Stats> =>
- parseChunked(createReadStream(filePath));
+export const readStatsFile = async (filePath: string): Promise<Stats> => {
+ if (existsSync(filePath)) {
+ return parseChunked(createReadStream(filePath));
+ }
+ return undefined;
+};
/**
* Utility to trim down a `preview-stats.json` file to the bare minimum, so that it can be used to

original_message: Update stats process to use stream


message: mock out readStats in upload process

diff:
import * as fs from 'fs-extra';
import progressStream from 'progress-stream';
-import * as readStatsFile from './read-stats-file';
import { getDependentStoryFiles as dependentStoryFiles } from '../lib/getDependentStoryFiles';
import { validateFiles, traceChangedFiles, uploadStorybook } from './upload';
jest.mock('fs-extra');
jest.mock('progress-stream');
jest.mock('../lib/getDependentStoryFiles');
+jest.mock('./read-stats-file');
const getDependentStoryFiles = <jest.MockedFunction<typeof dependentStoryFiles>>dependentStoryFiles;
const createReadStream = <jest.MockedFunction<typeof fs.createReadStream>>fs.createReadStream;
@@ -91,8 +91,6 @@ describe('validateFiles', () => {
});
describe('traceChangedFiles', () => {
- jest.spyOn(readStatsFile, 'readStatsFile').mockReturnValueOnce(Promise.resolve(undefined));
-
it('sets onlyStoryFiles on context', async () => {
const deps = { 123: ['./example.stories.js'] };
getDependentStoryFiles.mockResolvedValueOnce(deps);

original_message: mock out readStats in upload process


message: Update tests to correct state and to be more clear

diff:
@@ -438,8 +438,8 @@ describe('getDependentStoryFiles', () => {
expect.stringContaining(chalk`Found a static file change in {bold path/to/statics/image.png}`)
);
});
- it('ignores untraced files and dependencies', async () => {
- const changedFiles = ['src/stories/Button.jsx', 'src/stories/Page.jsx', 'build-storybook.log'];
+ it('ignores untraced files and dependencies with a glob pattern match', async () => {
+ const changedFiles = ['src/stories/Button.jsx', 'src/stories/Page.jsx'];
const modules = [
{
id: './src/stories/Button.jsx', // changed
@@ -559,41 +559,7 @@ describe('getDependentStoryFiles', () => {
},
{
id: './src/foo.js',
- name: './src/foo.js', // untraced
- reasons: [{ moduleName: './src/foo.stories.js' }],
- },
- {
- id: './src/foo.stories.js',
- name: './src/foo.stories.js',
- reasons: [{ moduleName: CSF_GLOB }],
- },
- {
- id: 997,
- name: CSF_GLOB,
- reasons: [{ moduleName: './.storybook/generated-stories-entry.js' }],
- },
- ];
- const ctx = getContext({
- untraced: ['**/package.json', '**/package-lock.json', '**/yarn.lock'],
- });
- const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
- expect(ctx.turboSnap.bailReason).toBeUndefined();
- expect(res).toEqual({
- './src/foo.stories.js': ['src/foo.stories.js'],
- });
- });
-
- it('does not bail on untraced Storybook config files', async () => {
- const changedFiles = ['src/utils.js', '.storybook/preview.js'];
- const modules = [
- {
- id: './src/utils.js',
- name: './src/utils.js', // changed
- reasons: [{ moduleName: './src/foo.js' }],
- },
- {
- id: './src/foo.js',
- name: './src/foo.js', // untraced
+ name: './src/foo.js',
reasons: [{ moduleName: './src/foo.stories.js' }],
},
{
@@ -608,7 +574,7 @@ describe('getDependentStoryFiles', () => {
},
];
const ctx = getContext({
- untraced: ['**/preview.js'],
+ untraced: ['**/(package**.json|yarn.lock)'],
});
const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
expect(ctx.turboSnap.bailReason).toBeUndefined();
@@ -617,23 +583,9 @@ describe('getDependentStoryFiles', () => {
});
});
- it('does not bail on changed wrapper component of untraced config file', async () => {
- const changedFiles = ['src/utils.js', 'src/docs-wrapper.jsx'];
+ it('does not bail on untraced Storybook config files that change due to a module sibling', async () => {
+ const changedFiles = ['src/utils.js', 'src/packages/design-system/components/button.jsx'];
const modules = [
- {
- id: './path/to/storybook-config/preview.js-generated-config-entry.js',
- name: './path/to/storybook-config/preview.js-generated-config-entry.js + 28 modules', // untraced
- modules: [
- { name: './path/to/storybook-config/preview.js-generated-config-entry.js' },
- { name: './src/docs-wrapper.jsx' },
- ],
- reasons: [],
- },
- {
- id: './src/docs-wrapper.jsx',
- name: './src/docs-wrapper.jsx', // untraced
- reasons: [{ moduleName: './src/docs-wrapper.stories.js' }],
- },
{
id: './src/utils.js',
name: './src/utils.js', // changed
@@ -649,72 +601,29 @@ describe('getDependentStoryFiles', () => {
name: './src/foo.stories.js',
reasons: [{ moduleName: CSF_GLOB }],
},
- {
- id: CSF_GLOB,
- name: CSF_GLOB,
- reasons: [{ moduleName: './path/to/storybook-config/generated-stories-entry.js' }],
- },
- ];
- const ctx = getContext({
- configDir: 'path/to/storybook-config',
- untraced: [
- '**/docs-wrapper.jsx',
- '**/path/to/storybook-config/preview.js-generated-config-entry.js',
- ],
- });
- const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
- expect(ctx.turboSnap.bailReason).toBeUndefined();
- expect(res).toEqual({
- './src/foo.stories.js': ['src/foo.stories.js'],
- });
- });
-
- it('does not bail on changed wrapper component of untraced config file and untraced dependencies of a wrapper component', async () => {
- const changedFiles = ['src/utils.js', 'src/packages/design-system/components/button.jsx'];
- const modules = [
- {
- id: './path/to/storybook-config/preview.js-generated-config-entry.js',
- name: './path/to/storybook-config/preview.js-generated-config-entry.js + 28 modules', // untraced
- modules: [
- { name: './path/to/storybook-config/preview.js-generated-config-entry.js' },
- { name: './src/docs-wrapper.jsx' },
- ],
- reasons: [],
- },
- {
- id: './src/docs-wrapper.jsx',
- name: './src/docs-wrapper.jsx',
- reasons: [{ moduleName: './src/packages/design-system/components/button.jsx' }],
- },
{
id: './src/packages/design-system/components/button.jsx',
name: './src/packages/design-system/components/button.jsx', // changed
- reasons: [{ moduleName: './src/packages/design-system/components/button.stories.js' }],
- },
- {
- id: './src/utils.js',
- name: './src/utils.js', // changed
- reasons: [{ moduleName: './src/foo.js' }],
+ reasons: [{ moduleName: './src/decorator.jsx' }],
},
{
- id: './src/foo.js',
- name: './src/foo.js',
- reasons: [{ moduleName: './src/foo.stories.js' }],
+ id: './src/packages/design-system/components/button.stories.js',
+ name: './src/packages/design-system/components/button.stories.js',
+ reasons: [{ moduleName: CSF_GLOB }],
},
{
- id: './src/foo.stories.js',
- name: './src/foo.stories.js',
+ id: './storybook/decorator.jsx',
+ name: './storybook/decorator.jsx', // untraced
reasons: [{ moduleName: CSF_GLOB }],
},
{
- id: CSF_GLOB,
+ id: 997,
name: CSF_GLOB,
- reasons: [{ moduleName: './path/to/storybook-config/generated-stories-entry.js' }],
+ reasons: [{ moduleName: './.storybook/generated-stories-entry.js' }],
},
];
const ctx = getContext({
- configDir: 'path/to/storybook-config',
- untraced: ['**/docs-wrapper.jsx'],
+ untraced: ['**/decorator.jsx'],
});
const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
expect(ctx.turboSnap.bailReason).toBeUndefined();
@@ -722,24 +631,9 @@ describe('getDependentStoryFiles', () => {
'./src/foo.stories.js': ['src/foo.stories.js'],
});
});
-
it('does not bail on changed dependency in dynamic import of untraced config file', async () => {
const changedFiles = ['src/utils.js', 'src/packages/design-system/components/button.jsx'];
const modules = [
- {
- id: './path/to/storybook-config/preview.js-generated-config-entry.js',
- name: './path/to/storybook-config/preview.js-generated-config-entry.js + 28 modules', // untraced
- modules: [
- { name: './path/to/storybook-config/preview.js-generated-config-entry.js' },
- { name: './src/docs-wrapper.jsx' },
- ],
- reasons: [],
- },
- {
- id: './src/docs-wrapper.jsx',
- name: './src/docs-wrapper.jsx',
- reasons: [{ moduleName: './src/packages/website/containers/ weak ^\\.\\/.*\\/index$' }],
- },
{
id: './packages/website/containers/ weak ^\\.\\/.*\\/index$',
name: './packages/website/containers/ weak ^\\.\\/.*\\/index$',
@@ -747,8 +641,21 @@ describe('getDependentStoryFiles', () => {
},
{
id: './src/packages/design-system/components/button.jsx',
- name: './src/packages/design-system/components/button.jsx', // changed
- reasons: [{ moduleName: './src/packages/design-system/components/button.stories.js' }],
+ name: './src/packages/design-system/components/button.jsx + 1 modules', // changed
+ reasons: [
+ { moduleName: './src/decorator.jsx' },
+ { moduleName: './src/packages/design-system/components/button.stories.js' },
+ ],
+ },
+ {
+ id: './src/packages/design-system/components/button.stories.js',
+ name: './src/packages/design-system/components/button.stories.js',
+ reasons: [{ moduleName: CSF_GLOB }],
+ },
+ {
+ id: './storybook/decorator.jsx',
+ name: './storybook/decorator.jsx', // untraced
+ reasons: [{ moduleName: CSF_GLOB }],
},
{
id: './src/utils.js',
@@ -773,7 +680,7 @@ describe('getDependentStoryFiles', () => {
];
const ctx = getContext({
configDir: 'path/to/storybook-config',
- untraced: ['**/docs-wrapper.jsx'],
+ untraced: ['**/decorator.jsx'],
});
const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
expect(ctx.turboSnap.bailReason).toBeUndefined();


message: Add optional chaining to builder check

diff:
@@ -157,7 +157,7 @@ const findConfigFlags = async ({ options, packageJson }) => {
export const findBuilder = async (mainConfig) => {
let name = 'webpack4'; // default builder in Storybook v6
- if (mainConfig?.core.builder) {
+ if (mainConfig?.core?.builder) {
const { builder } = mainConfig.core;
name = typeof builder === 'string' ? builder : builder.name;
}

original_message: Add optional chaining to builder check


message: use promise.allSettled vs promise.all in sb metadata gathering

diff:
@@ -175,7 +175,7 @@ export const getStorybookMetadata = async (ctx: Context) => {
const r = typeof __non_webpack_require__ !== 'undefined' ? __non_webpack_require__ : require;
const mainConfig = await r(path.resolve(configDir, 'main'));
- const info = await Promise.all([
+ const info = await Promise.allSettled([
findAddons(ctx, mainConfig),
findConfigFlags(ctx),
findViewlayer(ctx),


message: add optional chaining when there is issue with finding addon

diff:
@@ -5,6 +5,7 @@ import { initial, pending, success } from '../ui/tasks/storybookInfo';
export const setStorybookInfo = async (ctx: Context) => {
ctx.storybook = (await getStorybookInfo(ctx)) as Context['storybook'];
+ ctx.log.debug(ctx.storybook);
};
export default createTask({

diff:
@@ -11,7 +11,7 @@ const infoMessage = ({ addons, version, viewLayer, builder }: Context['storybook
const builderInfo = builder
? `${info}; using the ${builder.name} builder (${builder.packageVersion})`
: info;
- const supportedAddons = addons.filter((addon) => addon && addon.name);
+ const supportedAddons = addons?.filter((addon) => addon && addon.name);
return supportedAddons?.length
? `${builderInfo}; supported addons found: ${supportedAddons
.map((addon) => capitalize(addon.name))


message: determine what the addons is looking like

diff:
@@ -115,6 +115,7 @@ const findAddons = async (ctx, mainConfig) => {
...ctx.packageJson?.devDependencies,
...ctx.packageJson?.peerDependencies,
};
+ ctx.log.debug('addons, ', mainConfig.addons);
return {
addons: mainConfig.addons.map((addon) => {
let name: string;


message: log out the partial metadata promise

diff:
@@ -182,5 +182,6 @@ export const getStorybookMetadata = async (ctx: Context) => {
findViewlayer(ctx),
findBuilder(mainConfig),
]);
+ ctx.log.debug(info);
return info.reduce((acc, obj) => Object.assign(acc, obj), {});
};

diff:
@@ -5,7 +5,6 @@ import { initial, pending, success } from '../ui/tasks/storybookInfo';
export const setStorybookInfo = async (ctx: Context) => {
ctx.storybook = (await getStorybookInfo(ctx)) as Context['storybook'];
- ctx.log.debug(ctx.storybook);
};
export default createTask({

original_message: log out the partial metadata promise


message: improved logging of metadata

diff:
@@ -22,7 +22,8 @@ export default async function getStorybookInfo(
}
}
// Same for this await.
- return await getStorybookMetadata(ctx);
+ const sbMetadata = await getStorybookMetadata(ctx);
+ return sbMetadata;
} catch (e) {
ctx.log.debug(e);
return { viewLayer: null, version: null, addons: [], builder: null };

diff:
@@ -81,6 +81,7 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
// Note that `version` can be a semver range in this case.
return { viewLayer, version };
}
+ log.debug('viewLayer, ', `${viewLayer}`);
// Verify that the viewlayer package is actually present in node_modules.
return Promise.race([
resolvePackageJson(pkg)
@@ -115,7 +116,7 @@ const findAddons = async (ctx, mainConfig) => {
...ctx.packageJson?.devDependencies,
...ctx.packageJson?.peerDependencies,
};
- ctx.log.debug('addons, ', mainConfig.addons);
+
return {
addons: mainConfig.addons.map((addon) => {
let name: string;
@@ -156,13 +157,15 @@ const findConfigFlags = async ({ options, packageJson }) => {
};
};
-export const findBuilder = async (mainConfig) => {
+export const findBuilder = async (ctx, mainConfig) => {
let name = 'webpack4'; // default builder in Storybook v6
if (mainConfig?.core?.builder) {
const { builder } = mainConfig.core;
name = typeof builder === 'string' ? builder : builder.name;
}
+ ctx.log.debug('builder, ', name);
+
return Promise.race([
resolvePackageJson(builders[name])
.then((json) => ({ builder: { name, packageVersion: json.version } }))
@@ -180,7 +183,7 @@ export const getStorybookMetadata = async (ctx: Context) => {
findAddons(ctx, mainConfig),
findConfigFlags(ctx),
findViewlayer(ctx),
- findBuilder(mainConfig),
+ findBuilder(ctx, mainConfig),
]);
ctx.log.debug(info);
return info.reduce((acc, obj) => Object.assign(acc, obj), {});

diff:
@@ -11,7 +11,7 @@ const infoMessage = ({ addons, version, viewLayer, builder }: Context['storybook
const builderInfo = builder
? `${info}; using the ${builder.name} builder (${builder.packageVersion})`
: info;
- const supportedAddons = addons?.filter((addon) => addon && addon.name);
+ const supportedAddons = addons?.filter((addon) => addon?.name);
return supportedAddons?.length
? `${builderInfo}; supported addons found: ${supportedAddons
.map((addon) => capitalize(addon.name))

original_message: improved logging of metadata


message: troubleshoot viewLayer and builder finding

diff:
@@ -22,8 +22,7 @@ export default async function getStorybookInfo(
}
}
// Same for this await.
- const sbMetadata = await getStorybookMetadata(ctx);
- return sbMetadata;
+ return await getStorybookMetadata(ctx);
} catch (e) {
ctx.log.debug(e);
return { viewLayer: null, version: null, addons: [], builder: null };

diff:
@@ -81,11 +81,15 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
// Note that `version` can be a semver range in this case.
return { viewLayer, version };
}
- log.debug('viewLayer, ', `${viewLayer}`);
+ log.debug('viewLayer, ', `${viewLayer}: ${pkg}`);
+
// Verify that the viewlayer package is actually present in node_modules.
return Promise.race([
resolvePackageJson(pkg)
- .then((json) => ({ viewLayer, version: json.version }))
+ .then((json) => {
+ log.debug(json);
+ return { viewLayer, version: json.version };
+ })
.catch(() => Promise.reject(new Error(packageDoesNotExist(pkg)))),
timeout(10000),
]);


message: log error from catch blocks

diff:
@@ -90,7 +90,10 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
log.debug(json);
return { viewLayer, version: json.version };
})
- .catch(() => Promise.reject(new Error(packageDoesNotExist(pkg)))),
+ .catch((e) => {
+ log.debug(e);
+ return Promise.reject(new Error(packageDoesNotExist(pkg)));
+ }),
timeout(10000),
]);
}
@@ -173,7 +176,10 @@ export const findBuilder = async (ctx, mainConfig) => {
return Promise.race([
resolvePackageJson(builders[name])
.then((json) => ({ builder: { name, packageVersion: json.version } }))
- .catch(() => Promise.reject(new Error(packageDoesNotExist(builders[name])))),
+ .catch((e) => {
+ ctx.log.debug(e);
+ return Promise.reject(new Error(packageDoesNotExist(builders[name])));
+ }),
timeout(10000),
]);
};

original_message: log error from catch blocks


message: add logging to package resolution

diff:
@@ -11,13 +11,14 @@ import { timeout, raceFulfilled } from './promises';
import { supportedAddons } from './supportedAddons';
import { builders } from './builders';
-export const resolvePackageJson = (pkg: string) => {
+export const resolvePackageJson = (log, pkg: string) => {
try {
// we bundle this app for node, meaning all require calls are replaced by webpack.
// in this case we want to use node's actual require functionality!
// webpack will provide a '__non_webpack_require__' function to do this with,
// but this will obviously not be present during tests, hence the check and fallback to the normal require
const r = typeof __non_webpack_require__ !== 'undefined' ? __non_webpack_require__ : require;
+ log.debug(pkg);
const packagePath = r.resolve(`${pkg}/package.json`);
return fs.readJson(packagePath);
} catch (error) {
@@ -85,7 +86,7 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
// Verify that the viewlayer package is actually present in node_modules.
return Promise.race([
- resolvePackageJson(pkg)
+ resolvePackageJson(log, pkg)
.then((json) => {
log.debug(json);
return { viewLayer, version: json.version };
@@ -108,7 +109,7 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
return Promise.race([
raceFulfilled(
Object.entries(viewLayers).map(async ([key, value]) => {
- const json = await resolvePackageJson(key);
+ const json = await resolvePackageJson(log, key);
return { viewLayer: value, version: json.version };
})
).catch(() => Promise.reject(new Error(packageDoesNotExist(pkg)))),
@@ -164,20 +165,20 @@ const findConfigFlags = async ({ options, packageJson }) => {
};
};
-export const findBuilder = async (ctx, mainConfig) => {
+export const findBuilder = async ({ log }, mainConfig) => {
let name = 'webpack4'; // default builder in Storybook v6
if (mainConfig?.core?.builder) {
const { builder } = mainConfig.core;
name = typeof builder === 'string' ? builder : builder.name;
}
- ctx.log.debug('builder, ', name);
+ log.debug('builder, ', name);
return Promise.race([
- resolvePackageJson(builders[name])
+ resolvePackageJson(log, builders[name])
.then((json) => ({ builder: { name, packageVersion: json.version } }))
.catch((e) => {
- ctx.log.debug(e);
+ log.debug(e);
return Promise.reject(new Error(packageDoesNotExist(builders[name])));
}),
timeout(10000),

original_message: add logging to package resolution


message: point at node_mules for package resolution is using weird require

diff:
@@ -18,8 +18,12 @@ export const resolvePackageJson = (log, pkg: string) => {
// webpack will provide a '__non_webpack_require__' function to do this with,
// but this will obviously not be present during tests, hence the check and fallback to the normal require
const r = typeof __non_webpack_require__ !== 'undefined' ? __non_webpack_require__ : require;
- log.debug(pkg);
- const packagePath = r.resolve(`${pkg}/package.json`);
+ const pkgPath =
+ typeof __non_webpack_require__ !== 'undefined'
+ ? `./node_modules/${pkg}/package.json`
+ : `${pkg}/package.json`;
+ log.debug(pkgPath);
+ const packagePath = r.resolve(pkgPath);
return fs.readJson(packagePath);
} catch (error) {
return Promise.reject(error);


message: use path.resolve instead of require.resolve

diff:
@@ -11,19 +11,9 @@ import { timeout, raceFulfilled } from './promises';
import { supportedAddons } from './supportedAddons';
import { builders } from './builders';
-export const resolvePackageJson = (log, pkg: string) => {
+export const resolvePackageJson = (pkg: string) => {
try {
- // we bundle this app for node, meaning all require calls are replaced by webpack.
- // in this case we want to use node's actual require functionality!
- // webpack will provide a '__non_webpack_require__' function to do this with,
- // but this will obviously not be present during tests, hence the check and fallback to the normal require
- const r = typeof __non_webpack_require__ !== 'undefined' ? __non_webpack_require__ : require;
- const pkgPath =
- typeof __non_webpack_require__ !== 'undefined'
- ? `./node_modules/${pkg}/package.json`
- : `${pkg}/package.json`;
- log.debug(pkgPath);
- const packagePath = r.resolve(pkgPath);
+ const packagePath = path.join(__dirname, `./node_modules/${pkg}/package.json`);
return fs.readJson(packagePath);
} catch (error) {
return Promise.reject(error);
@@ -90,7 +80,7 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
// Verify that the viewlayer package is actually present in node_modules.
return Promise.race([
- resolvePackageJson(log, pkg)
+ resolvePackageJson(pkg)
.then((json) => {
log.debug(json);
return { viewLayer, version: json.version };
@@ -113,7 +103,7 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
return Promise.race([
raceFulfilled(
Object.entries(viewLayers).map(async ([key, value]) => {
- const json = await resolvePackageJson(log, key);
+ const json = await resolvePackageJson(key);
return { viewLayer: value, version: json.version };
})
).catch(() => Promise.reject(new Error(packageDoesNotExist(pkg)))),
@@ -179,7 +169,7 @@ export const findBuilder = async ({ log }, mainConfig) => {
log.debug('builder, ', name);
return Promise.race([
- resolvePackageJson(log, builders[name])
+ resolvePackageJson(builders[name])
.then((json) => ({ builder: { name, packageVersion: json.version } }))
.catch((e) => {
log.debug(e);


message: use path.resolve on package name

diff:
@@ -11,9 +11,15 @@ import { timeout, raceFulfilled } from './promises';
import { supportedAddons } from './supportedAddons';
import { builders } from './builders';
-export const resolvePackageJson = (pkg: string) => {
+export const resolvePackageJson = (log, pkg: string) => {
try {
- const packagePath = path.join(__dirname, `./node_modules/${pkg}/package.json`);
+ // we bundle this app for node, meaning all require calls are replaced by webpack.
+ // in this case we want to use node's actual require functionality!
+ // webpack will provide a '__non_webpack_require__' function to do this with,
+ // but this will obviously not be present during tests, hence the check and fallback to the normal require
+ // const r = typeof __non_webpack_require__ !== 'undefined' ? __non_webpack_require__ : require;
+ log.debug(pkg);
+ const packagePath = path.resolve(`${pkg}/package.json`);
return fs.readJson(packagePath);
} catch (error) {
return Promise.reject(error);
@@ -80,7 +86,7 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
// Verify that the viewlayer package is actually present in node_modules.
return Promise.race([
- resolvePackageJson(pkg)
+ resolvePackageJson(log, pkg)
.then((json) => {
log.debug(json);
return { viewLayer, version: json.version };
@@ -103,7 +109,7 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
return Promise.race([
raceFulfilled(
Object.entries(viewLayers).map(async ([key, value]) => {
- const json = await resolvePackageJson(key);
+ const json = await resolvePackageJson(log, key);
return { viewLayer: value, version: json.version };
})
).catch(() => Promise.reject(new Error(packageDoesNotExist(pkg)))),
@@ -169,7 +175,7 @@ export const findBuilder = async ({ log }, mainConfig) => {
log.debug('builder, ', name);
return Promise.race([
- resolvePackageJson(builders[name])
+ resolvePackageJson(log, builders[name])
.then((json) => ({ builder: { name, packageVersion: json.version } }))
.catch((e) => {
log.debug(e);

original_message: use path.resolve on package name


message: add to resolvePackageJson path

diff:
@@ -19,7 +19,7 @@ export const resolvePackageJson = (log, pkg: string) => {
// but this will obviously not be present during tests, hence the check and fallback to the normal require
// const r = typeof __non_webpack_require__ !== 'undefined' ? __non_webpack_require__ : require;
log.debug(pkg);
- const packagePath = path.resolve(`${pkg}/package.json`);
+ const packagePath = path.resolve(`node_modules/${pkg}/package.json`);
return fs.readJson(packagePath);
} catch (error) {
return Promise.reject(error);

original_message: add to resolvePackageJson path


message: remove some log statements

diff:
@@ -13,12 +13,6 @@ import { builders } from './builders';
export const resolvePackageJson = (log, pkg: string) => {
try {
- // we bundle this app for node, meaning all require calls are replaced by webpack.
- // in this case we want to use node's actual require functionality!
- // webpack will provide a '__non_webpack_require__' function to do this with,
- // but this will obviously not be present during tests, hence the check and fallback to the normal require
- // const r = typeof __non_webpack_require__ !== 'undefined' ? __non_webpack_require__ : require;
- log.debug(pkg);
const packagePath = path.resolve(`node_modules/${pkg}/package.json`);
return fs.readJson(packagePath);
} catch (error) {
@@ -82,19 +76,12 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
// Note that `version` can be a semver range in this case.
return { viewLayer, version };
}
- log.debug('viewLayer, ', `${viewLayer}: ${pkg}`);
// Verify that the viewlayer package is actually present in node_modules.
return Promise.race([
resolvePackageJson(log, pkg)
- .then((json) => {
- log.debug(json);
- return { viewLayer, version: json.version };
- })
- .catch((e) => {
- log.debug(e);
- return Promise.reject(new Error(packageDoesNotExist(pkg)));
- }),
+ .then((json) => ({ viewLayer, version: json.version }))
+ .catch(() => Promise.reject(new Error(packageDoesNotExist(pkg)))),
timeout(10000),
]);
}
@@ -172,15 +159,10 @@ export const findBuilder = async ({ log }, mainConfig) => {
name = typeof builder === 'string' ? builder : builder.name;
}
- log.debug('builder, ', name);
-
return Promise.race([
resolvePackageJson(log, builders[name])
.then((json) => ({ builder: { name, packageVersion: json.version } }))
- .catch((e) => {
- log.debug(e);
- return Promise.reject(new Error(packageDoesNotExist(builders[name])));
- }),
+ .catch(() => Promise.reject(new Error(packageDoesNotExist(builders[name])))),
timeout(10000),
]);
};

diff:
@@ -5,6 +5,7 @@ import { initial, pending, success } from '../ui/tasks/storybookInfo';
export const setStorybookInfo = async (ctx: Context) => {
ctx.storybook = (await getStorybookInfo(ctx)) as Context['storybook'];
+ ctx.log.debug('storybook', ctx.storybook);
};
export default createTask({

original_message: remove some log statements


message: remove promiseSettled

diff:
@@ -172,12 +172,12 @@ export const getStorybookMetadata = async (ctx: Context) => {
const r = typeof __non_webpack_require__ !== 'undefined' ? __non_webpack_require__ : require;
const mainConfig = await r(path.resolve(configDir, 'main'));
- const info = await Promise.allSettled([
+ const info = await Promise.all([
findAddons(ctx, mainConfig),
findConfigFlags(ctx),
findViewlayer(ctx),
findBuilder(ctx, mainConfig),
]);
ctx.log.debug(info);
- return info.reduce((acc, obj) => Object.assign(acc, obj), {});
+ return Object.assign({}, ...info);
};

original_message: remove promiseSettled


message: remove rogue whitespace

diff:
@@ -76,7 +76,6 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
// Note that `version` can be a semver range in this case.
return { viewLayer, version };
}
-
// Verify that the viewlayer package is actually present in node_modules.
return Promise.race([
resolvePackageJson(pkg)
@@ -111,7 +110,6 @@ const findAddons = async (ctx, mainConfig) => {
...ctx.packageJson?.devDependencies,
...ctx.packageJson?.peerDependencies,
};
-
return {
addons: mainConfig.addons.map((addon) => {
let name: string;

original_message: remove rogue whitespace


message: remove unintentional debug log

diff:
@@ -176,6 +176,5 @@ export const getStorybookMetadata = async (ctx: Context) => {
findViewlayer(ctx),
findBuilder(mainConfig),
]);
- ctx.log.debug(info);
return info.reduce((acc, obj) => Object.assign(acc, obj), {});
};

original_message: remove unintentional debug log


message: brute force fix for ngResource query param in path

diff:
@@ -185,6 +185,38 @@ describe('getDependentStoryFiles', () => {
});
});
+ it('detects indirect changes to CSF files, angular css resource', async () => {
+ const changedFiles = ['src/foo.css'];
+ const modules = [
+ {
+ id: null,
+ name: './src/foo.css?ngResource ',
+ reasons: [{ moduleName: './src/foo.component.ts' }],
+ },
+ {
+ id: './src/foo.component.ts',
+ name: './src/foo.component.ts + 1 modules',
+ modules: [{ name: './src/foo.component.ts' }, { name: './src/foo.css?ngResource' }],
+ reasons: [{ moduleName: './src/foo.stories.ts' }],
+ },
+ {
+ id: './src/foo.stories.ts',
+ name: './src/foo.stories.ts',
+ reasons: [{ moduleName: CSF_GLOB }],
+ },
+ {
+ id: CSF_GLOB,
+ name: CSF_GLOB,
+ reasons: [{ moduleName: './.storybook/generated-stories-entry.js' }],
+ },
+ ];
+ const ctx = getContext();
+ const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
+ expect(res).toEqual({
+ './src/foo.stories.ts': ['src/foo.stories.ts'],
+ });
+ });
+
it('detects indirect changes to CSF files in a single module chunk', async () => {
const changedFiles = ['src/foo.js'];
const modules = [

diff:
@@ -97,13 +97,18 @@ export async function getDependentStoryFiles(
stats.modules
.filter((mod) => isUserModule(mod))
.forEach((mod) => {
- const normalizedName = normalize(mod.name);
+ let { name } = mod;
+ if (mod.name.includes('?ngResource')) {
+ [name] = mod.name.split('?');
+ }
+ const normalizedName = normalize(name);
modulesByName[normalizedName] = mod;
namesById[mod.id] = normalizedName;
if (mod.modules) {
mod.modules.forEach((m) => {
- modulesByName[normalize(m.name)] = mod;
+ modulesByName[normalize(m.name.includes('?ngResource') ? m.name.split('?')[0] : m.name)] =
+ mod;
});
}


message: add regex for replaciing url parameters

diff:
@@ -93,13 +93,14 @@ export async function getDependentStoryFiles(
const namesById: Record<number, string> = {};
const reasonsById: Record<number, string[]> = {};
const csfGlobsByName: Record<string, true> = {};
-
+ const CSF_REGEX = /.\/src.*?sync/g;
+ const URL_PARAM_REGEX = /(\?.*)|(#.*)/g;
stats.modules
.filter((mod) => isUserModule(mod))
.forEach((mod) => {
let { name } = mod;
- if (mod.name.includes('?ngResource')) {
- [name] = mod.name.split('?');
+ if (URL_PARAM_REGEX.test(mod.name) && !CSF_REGEX.test(mod.name)) {
+ [name] = mod.name.replace(URL_PARAM_REGEX, '');
}
const normalizedName = normalize(name);
modulesByName[normalizedName] = mod;
@@ -107,8 +108,13 @@ export async function getDependentStoryFiles(
if (mod.modules) {
mod.modules.forEach((m) => {
- modulesByName[normalize(m.name.includes('?ngResource') ? m.name.split('?')[0] : m.name)] =
- mod;
+ modulesByName[
+ normalize(
+ URL_PARAM_REGEX.test(m.name) && !CSF_REGEX.test(m.name)
+ ? m.name.replace(URL_PARAM_REGEX, '')
+ : m.name
+ )
+ ] = mod;
});
}

diff:
{
"exclude": ["node_modules"],
"compilerOptions": {
- "target": "es5",
+ "target": "ES6",
"module": "commonjs",
"moduleResolution": "node",
"noImplicitAny": false,
"esModuleInterop": true,
"resolveJsonModule": true,
- "types": [
- "jest",
- "webpack-env"
- ]
+ "types": ["jest", "webpack-env"]
}
}

original_message: add regex for replaciing url parameters


message: add a nice comment

diff:
@@ -99,6 +99,10 @@ export async function getDependentStoryFiles(
.filter((mod) => isUserModule(mod))
.forEach((mod) => {
let { name } = mod;
+ // This regex test is to ensure file names do not include url parmeters
+ // or match the CSF glob we get back in the stats file. We added this because
+ // CSS/SCSS files were getting ?ngResource appended on the end of file names
+ // in the stats file.
if (URL_PARAM_REGEX.test(mod.name) && !CSF_REGEX.test(mod.name)) {
[name] = mod.name.replace(URL_PARAM_REGEX, '');
}

original_message: add a nice comment


message: update normalize method

diff:
@@ -69,7 +69,20 @@ export async function getDependentStoryFiles(
// Convert a "webpack path" (relative to storybookBaseDir) to a "git path" (relative to repository root)
// e.g. `./src/file.js` => `path/to/storybook/src/file.js`
- const normalize = (posixPath: string) => normalizePath(posixPath, rootPath, baseDir);
+ const normalize = (posixPath: string) => {
+ const CSF_REGEX = /.\/src.*?sync/g;
+ const URL_PARAM_REGEX = /(\?.*)|(#.*)/g;
+ let newPath = normalizePath(posixPath, rootPath, baseDir);
+ // This regex test is to ensure file names do not include url parmeters
+ // or match the CSF glob we get back in the stats file. We added this because
+ // CSS/SCSS files were getting ?ngResource appended on the end of file names
+ // in the stats file.
+ if (URL_PARAM_REGEX.test(newPath) && !CSF_REGEX.test(newPath)) {
+ newPath = newPath.replace(URL_PARAM_REGEX, '');
+ }
+
+ return newPath;
+ };
const storybookDir = normalize(posix(storybookConfigDir));
const staticDirs = staticDir.map((dir: string) => normalize(posix(dir)));
@@ -93,32 +106,18 @@ export async function getDependentStoryFiles(
const namesById: Record<number, string> = {};
const reasonsById: Record<number, string[]> = {};
const csfGlobsByName: Record<string, true> = {};
- const CSF_REGEX = /.\/src.*?sync/g;
- const URL_PARAM_REGEX = /(\?.*)|(#.*)/g;
+
stats.modules
.filter((mod) => isUserModule(mod))
.forEach((mod) => {
- let { name } = mod;
- // This regex test is to ensure file names do not include url parmeters
- // or match the CSF glob we get back in the stats file. We added this because
- // CSS/SCSS files were getting ?ngResource appended on the end of file names
- // in the stats file.
- if (URL_PARAM_REGEX.test(mod.name) && !CSF_REGEX.test(mod.name)) {
- [name] = mod.name.replace(URL_PARAM_REGEX, '');
- }
+ const { name } = mod;
const normalizedName = normalize(name);
modulesByName[normalizedName] = mod;
namesById[mod.id] = normalizedName;
if (mod.modules) {
mod.modules.forEach((m) => {
- modulesByName[
- normalize(
- URL_PARAM_REGEX.test(m.name) && !CSF_REGEX.test(m.name)
- ? m.name.replace(URL_PARAM_REGEX, '')
- : m.name
- )
- ] = mod;
+ modulesByName[normalize(m.name)] = mod;
});
}

original_message: update normalize method


diff:
@@ -73,7 +73,7 @@ export async function getDependentStoryFiles(
const CSF_REGEX = /\s+sync\s+/g;
const URL_PARAM_REGEX = /(\?.*)/g;
let newPath = normalizePath(posixPath, rootPath, baseDir);
- // This regex test is to ensure file names do not include url parmeters
+ // This regex test is to ensure file names do not include url parameters
// or match the CSF glob we get back in the stats file. We added this because
// CSS/SCSS files were getting ?ngResource appended on the end of file names
// in the stats file.

original_message:
Update comment in bin-src/lib/getDependentStoryFiles.ts

Co-authored-by: Tom Coleman <tom@thesnail.org>


message: Added an example for a TurboSnap optimized flow

diff:
+name: Chromatic
+on:
+ push:
+ branches:
+ - main
+ pull_request:
+ types: [assigned, ready_for_review, review_requested]
+
+jobs:
+ chromatic:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ fetch-depth: 0
+ - name: install
+ run: yarn install --frozen-lockfile
+ - uses: chromaui/action-next@v1
+ with:
+ exitOnceUploaded: true
+ onlyChanged: true
+ traceChanged: true
+ diagnostics: true
+ # Only use this if you notice too many builds with TurboSnap exiting due to a global package.json, package-lock.json, yarn.lock
+ # Typically for use when you have an active repository with lots of rebasing or squash and merge commits
+ # Documentation https://www.chromatic.com/docs/turbosnap#avoid-re-testing-on-changes-to-package-control-files
+ # This pattern will ignore all of the following files (package.json src/app/package.json yarn.lock app/yarn.lock)
+ untraced: |
+ - '**/(package**.json|yarn.lock)'
+ env:
+ CHROMATIC_PROJECT_TOKEN: gcaw1ai2dgo
+ - uses: actions/upload-artifact@v2
+ if: always()
+ with:
+ name: chromatic-build-artifacts-${{ github.run_id }}
+ path: |
+ chromatic-diagnostics.json
+ **/build-storybook.log


message: Added onlyStoryFiles to the github action

diff:
@@ -147,6 +147,7 @@ async function run() {
const only = getInput('only');
const onlyChanged = getInput('onlyChanged');
const onlyStoryNames = getInput('onlyStoryNames');
+ const onlyStoryFiles = getInput('onlyStoryFiles');
const externals = getInput('externals');
const untraced = getInput('untraced');
const traceChanged = getInput('traceChanged');
@@ -191,6 +192,7 @@ async function run() {
only: maybe(only),
onlyChanged: maybe(onlyChanged),
onlyStoryNames: maybe(onlyStoryNames),
+ onlyStoryFiles: maybe(onlyStoryFiles),
externals: maybe(externals),
untraced: maybe(untraced),
storybookBaseDir: maybe(storybookBaseDir),

diff:
@@ -51,6 +51,9 @@ inputs:
onlyStoryNames:
description: 'Only run a single story or a subset of stories by their name'
required: false
+ onlyStoryFiles:
+ description: 'Only run a single story or a subset of stories by their filename(s)'
+ required: false
externals:
description: 'Disable TurboSnap when any of these files have changed since the baseline build'
required: false


message: "test commit for untraced"

diff:
@@ -15,10 +15,13 @@ const LOCKFILES = [
/\/yarn\.lock$/,
];
+const GLOBALS = [/^package\.json$/, /\/package\.json$/];
+
// Ignore these while tracing dependencies
const EXTERNALS = [/^node_modules\//, /\/node_modules\//, /\/webpack\/runtime\//, /^\(webpack\)/];
const isPackageLockFile = (name: string) => LOCKFILES.some((re) => re.test(name));
+const isPackageFile = (name: string) => GLOBALS.some((re) => re.test(name));
const isUserModule = (mod: Module | Reason) =>
(mod as Module).id !== undefined &&
(mod as Module).id !== null &&
@@ -183,7 +186,7 @@ export async function getDependentStoryFiles(
bailReason: undefined,
};
- const changedPackageFiles = tracedFiles.filter(isPackageLockFile);
+ const changedPackageFiles = tracedFiles.filter(isPackageLockFile || isPackageFile);
if (changedPackageFiles.length) {
ctx.turboSnap.bailReason = { changedPackageFiles };
// If package.json dependencies changed, we still want to use the same TurboSnap bail reason

diff:
"storybook",
"addon",
"test",
- "popular"
+ "popular",
+ "automation"
],
"homepage": "https://www.chromatic.com",
"bugs": {

diff:
"scripts": {
"start": "start-storybook",
"build": "build-storybook --webpack-stats-json --output-dir ../subdir-static",
- "postbuild": "node -r esm ../bin-src/trim-stats-file.js ../subdir-static/preview-stats.json"
+ "postbuild": "node -r esm ../bin-src/trim-stats-file.js ../subdir-static/preview-stats.json",
+ "test": "jest"
},
"dependencies": {
"@storybook/react": "^6.3.7",

original_message: "test commit for untraced"


message: Updated logic and added a test

diff:
@@ -395,6 +395,33 @@ describe('getDependentStoryFiles', () => {
);
});
+ it('does not bail on dependency changes to package manifest file when untraced', async () => {
+ const changedFiles = ['src/foo.stories.js', 'src/package.json', 'src/package-lock.json'];
+ const modules = [
+ {
+ id: './src/foo.stories.js',
+ name: './src/foo.stories.js',
+ reasons: [{ moduleName: CSF_GLOB }],
+ },
+ {
+ id: CSF_GLOB,
+ name: CSF_GLOB,
+ reasons: [{ moduleName: './.storybook/generated-stories-entry.js' }],
+ },
+ ];
+ const rawContext = getContext({ untraced: ['**/(package.json|package-lock.json|yarn.lock)'] });
+ const ctx = {
+ ...rawContext,
+ // signifying the package.json file had dependency changes
+ git: { ...rawContext.git, changedPackageManifests: ['src/package.json'] },
+ };
+ const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
+ expect(ctx.turboSnap.bailReason).toBeUndefined();
+ expect(res).toEqual({
+ './src/foo.stories.js': ['src/foo.stories.js'],
+ });
+ });
+
it('does not bail when package.json change has no dependency changes', async () => {
const changedFiles = ['src/foo.stories.js', 'src/package.json'];
const modules = [

diff:
@@ -186,12 +186,12 @@ export async function getDependentStoryFiles(
bailReason: undefined,
};
- const changedPackageFiles = tracedFiles.filter(isPackageLockFile || isPackageFile);
+ const changedPackageFiles = tracedFiles.filter(isPackageLockFile);
if (changedPackageFiles.length) {
ctx.turboSnap.bailReason = { changedPackageFiles };
// If package.json dependencies changed, we still want to use the same TurboSnap bail reason
// for now.
- } else if (ctx.git.changedPackageManifests?.length) {
+ } else if (ctx.git.changedPackageManifests?.length && tracedFiles.filter(isPackageFile).length) {
ctx.turboSnap.bailReason = { changedPackageFiles: ctx.git.changedPackageManifests };
}

original_message: Updated logic and added a test


message: Improve the failure message when there's only one git commit

diff:
@@ -8,14 +8,20 @@ export default (isGithubAction = false) =>
isGithubAction
? dedent(chalk`
${error} {bold Found only one commit}
- This typically means you've checked out a shallow copy of the Git repository, which {bold actions/checkout@v2} does by default.
+ This typically means you have ran into one of the following scenarios:
+ - You've checked out a shallow copy of the Git repository, which {bold actions/checkout@v2} does by default.
In order for Chromatic to correctly determine baseline commits, we need access to the full Git history graph.
With {bold actions/checkout@v2}, you can enable this by setting 'fetch-depth: 0'.
${info} Read more at ${link('https://www.chromatic.com/docs/github-actions')}
+ - You've only made a single commit so far.
+ Chromatic needs an additional commit to be able to detect what's changed.
`)
: dedent(chalk`
${error} {bold Found only one commit}
- This typically means you've checked out a shallow copy of the Git repository, which some CI systems do by default.
+ This typically means you have ran into one of the following scenarios:
+ - You've checked out a shallow copy of the Git repository, which some CI systems do by default.
In order for Chromatic to correctly determine baseline commits, we need access to the full Git history graph.
Refer to your CI provider's documentation for details.
+ - You've only made a single commit so far.
+ Chromatic needs an additional commit to be able to detect what's changed.
`);


message: add latest canary tag

diff:
{
"name": "chromatic",
- "version": "6.16.0-canary.0",
+ "version": "6.16.0-canary.3",
"description": "Automate visual testing across browsers. Gather UI feedback. Versioned documentation.",
"keywords": [
"storybook-addon",

original_message: add latest canary tag


message: Add logs during build packaging

diff:
@@ -92,6 +92,10 @@ function package_mac_version() {
set_env
prepare_mac_resources_for_build_platform_script
WORKSPACE=$(pwd)/One ./pharo-build-scripts/build-platform.sh -i Pharo -o PharoLauncher -r $PHARO -s $PHARO_SOURCES -v $VERSION-$DATE -t PharoLauncher -p mac
+ echo "PRINT in package_mac_version"
+ pwd
+ ls -r
+ echo "End PRINT in package_mac_version"
unzip PharoLauncher-mac.zip -d .
mv mac-installer-background.png background.png
@@ -105,6 +109,10 @@ function package_mac_version() {
function package_windows_version() {
set_env
bash ./pharo-build-scripts/build-platform.sh -i Pharo -o Pharo -r $PHARO -s $PHARO_SOURCES -v $VERSION-$DATE -t Pharo -p win
+ echo "PRINT in package_mac_version"
+ pwd
+ ls -r
+ echo "End PRINT in package_mac_version"
unzip Pharo-win.zip -d .
openssl aes-256-cbc -k "${PHARO_CERT_PASSWORD}" -in signing/pharo-windows-certificate.p12.enc -out pharo-windows-certificate.p12 -d

repo: pharo-project/pharo-launcher

original_message: Add logs during build packaging


message: Recursive ls

diff:
@@ -94,7 +94,7 @@ function package_mac_version() {
WORKSPACE=$(pwd)/One ./pharo-build-scripts/build-platform.sh -i Pharo -o PharoLauncher -r $PHARO -s $PHARO_SOURCES -v $VERSION-$DATE -t PharoLauncher -p mac
echo "PRINT in package_mac_version"
pwd
- ls -r
+ ls -R
echo "End PRINT in package_mac_version"
unzip PharoLauncher-mac.zip -d .
mv mac-installer-background.png background.png
@@ -111,7 +111,7 @@ function package_windows_version() {
bash ./pharo-build-scripts/build-platform.sh -i Pharo -o Pharo -r $PHARO -s $PHARO_SOURCES -v $VERSION-$DATE -t Pharo -p win
echo "PRINT in package_mac_version"
pwd
- ls -r
+ ls -R
echo "End PRINT in package_mac_version"
unzip Pharo-win.zip -d .

original_message: Recursive ls


message: fix unzipping

diff:
@@ -96,7 +96,7 @@ function package_mac_version() {
pwd
ls -R
echo "End PRINT in package_mac_version"
- unzip PharoLauncher-mac.zip -d .
+ unzip ./One/PharoLauncher-mac.zip -d .
mv mac-installer-background.png background.png
VERSION=$VERSION_NUMBER APP_NAME=PharoLauncher ./mac/build-dmg.sh

original_message: fix unzipping


message: save changes of DamienC

diff:
@@ -8,11 +8,11 @@ fetchOnlineImage: anURL
xmlDoc allElementsNamed: #name do: [ :each | name add: (each nodes first name) ] .
xmlDoc allElementsNamed: #url do: [ :each | url add: (each nodes first name) ] .
- name do: [ :each |
+ name with: url do: [ :eachName :eachUrl |
self pharoVersions do: [ :element |
- Transcript cr;show: element asString, ' - ', each asString.
- ((each findString: element) = 1)
- ifTrue: [ tdictionary at: each put: (url at:(name find: each))asString, 'lastSuccessfulBuild/artifact/', each asString, '.zip']].
+ Transcript cr;show: element asString, ' - ', eachName asString.
+ (eachName beginsWith: element)
+ ifTrue: [ tdictionary at: eachName put: eachUrl asString, 'lastSuccessfulBuild/artifact/', eachName asString, '.zip']].
].
^tdictionary

original_message: save changes of DamienC


message: basic stuff is working

diff:
+initialization
+loadingModel: aString
+
+ self primaryUI loadModel: aString
+

diff:
@@ -4,5 +4,5 @@ setLoadingModel: aString
loadingButtonModel
state: false;
label: 'Loading ', aString;
- action: [ self refreshModel ].
+ action: [ self loadingModel: aString ].

diff:
instance creation
initialize
^ self new
- url: 'https://gitorious.org/pharo-build/pharo-build/blobs/raw/aebcfb9a214ed8aab98c3c944828dd7786eabf21/pharo-templates/template.xml';
+ url: 'https://gitorious.org/pharo-build/pharo-build/blobs/raw/master/pharo-templates/template.xml';
fetchOnlineImage.
\ No newline at end of file

diff:
+runnnig
+load: aString
+ self listElement do: [ :each | Transcript cr; show: aString, '- ', each name asString.(each name asString = aString) ifTrue: [ self loadAndRun: each ] ].
\ No newline at end of file

diff:
+runnnig
+loadAndRun: anElement
+ self download: anElement url asString.
+ self unzip: anElement url asString asZnUrl lastPathSegment .
+ self run: anElement name asString, '.image'.
\ No newline at end of file

change_type: DELETE

diff:
-runnnig
-loadAndRun
- self download: (self listElement at: self listModel selectedItem).
- self unzip: (self listModel selectedItem).
\ No newline at end of file

diff:
+runnnig
+run: anElement
+ self halt.
+ OSProcess command:
+ (Smalltalk vm fullPath asString,
+ ' ',
+ FileSystem disk workingDirectory pathString,
+ '\',
+ anElement asString).
\ No newline at end of file

diff:
unzipping
-unzip: anURL
-
- GZipReadStream
- saveContents: (FileSystem disk workingDirectory/ anURL, 'zip' ) fullName.
\ No newline at end of file
+unzip: aFileName
+ ZipArchive
+ extractAllIn: (FileSystem disk workingDirectory/ aFileName) fullName.
\ No newline at end of file

diff:
+accessing
+loadModel: aString
+ self model load: aString
\ No newline at end of file

original_message: basic stuff is working


message:
Now reuse past .zip
Now ask for a project name

diff:
downloading
-download: anURL
+download: anElement
+ | fileToDownload |
+ fileToDownload := (StartupLoader preferencesGeneralFolder asFileReference / SystemVersion current dottedMajorMinor / anElement name, 'zip').
+ (fileToDownload exists)
+ ifTrue: [ ^ self ]
+ ifFalse: [ ZnClient new
+ url: anElement url asString ;
+ downloadTo: fileToDownload . ].
\ No newline at end of file
- ZnClient new
- url: anURL ;
- downloadTo: (StartupLoader preferencesGeneralFolder asFileReference / SystemVersion current dottedMajorMinor).
\ No newline at end of file

diff:
@@ -5,7 +5,7 @@ fetchOnlineImage: anURL
list := OrderedCollection new.
xmlDoc allElementsNamed: #template do: [:each |
list add: (ImageLauncherElement
- name: (each nodes elementAt: #name) nodes first
- url: (each nodes elementAt: #url) nodes first
- category: (each nodes elementAt: #category) nodes first)].
+ name: (each nodes elementAt: #name) nodes first asString
+ url: (each nodes elementAt: #url) nodes first asString
+ category: (each nodes elementAt: #category) nodes first asString )].
^ list
\ No newline at end of file

diff:
+unzipping
+imageDirectory
+ ^ (StartupLoader preferencesGeneralFolder asFileReference / SystemVersion current dottedMajorMinor)
\ No newline at end of file

diff:
runnnig
loadAndRun: anElement
- self download: anElement url asString.
- self unzip: anElement url asString asZnUrl lastPathSegment .
- self run: anElement name asString, '.image'.
\ No newline at end of file
+ self download: anElement .
+ self unzip: anElement .
+ self run: (self userChoiceName: anElement)
\ No newline at end of file

diff:
+runnnig
+move: aStringFile from: aStringOrigninalDirectory to: aStringDirectory
+
+ OSProcess command:
+ ('mv ',
+ aStringOrigninalDirectory ,
+ '/',
+ aStringFile,
+ ' ',
+ aStringDirectory ,
+ '/').
\ No newline at end of file

diff:
+runnnig
+move: aStringFile to: aStringDirectory
+
+ | aString |
+ OSProcess command:
+ ('mv ',
+ aStringDirectory,
+ '/*.image ',
+ aStringDirectory ,
+ '/',
+ aString, '.image').
+ OSProcess command:
+ ('mv ',
+ aStringDirectory,
+ '/*.changes ',
+ aStringDirectory ,
+ '/',
+ aString, '.changes').
\ No newline at end of file

diff:
+runnnig
+rename: aStringDirectory to: aString
+
+ OSProcess command:
+ ('mv ',
+ aStringDirectory,
+ '/*.image ',
+ aStringDirectory ,
+ '/',
+ aString, '.image').
+ OSProcess command:
+ ('mv ',
+ aStringDirectory,
+ '/*.changes ',
+ aStringDirectory ,
+ '/',
+ aString, '.changes').
\ No newline at end of file

diff:
runnnig
-run: anElement
+run: aString
+ self halt.
OSProcess command:
(Smalltalk vm fullPath asString,
' ',
- FileSystem disk workingDirectory pathString,
- '\',
- anElement asString).
\ No newline at end of file
+ self imageDirectory pathString,
+ '/',
+ aString,
+ '.image').
\ No newline at end of file

diff:
+runnnig
+tmpDirectory: anElement
+ ^ (StartupLoader preferencesGeneralFolder asFileReference / SystemVersion current dottedMajorMinor / anElement name asString ) pathString
\ No newline at end of file

diff:
unzipping
-unzip: aFileName
+unzip: anElement
ZipArchive new
- readFrom: (StartupLoader preferencesGeneralFolder asFileReference / SystemVersion current dottedMajorMinor / aFileName);
- extractAllTo: (StartupLoader preferencesGeneralFolder asFileReference / SystemVersion current dottedMajorMinor).
\ No newline at end of file
+ readFrom: (self imageDirectory / anElement name asString, 'zip');
+ extractAllTo: (self imageDirectory / anElement name asString ).
+ self rename: (self tmpDirectory: anElement) to: anElement name asString
\ No newline at end of file

diff:
+runnnig
+userChoiceName: anElement
+ | widget |
+ widget := InputWidget new.
+ widget
+ title: 'Choice project name';
+ label: 'Project name?';
+ ghostText: ''.
+
+ World openModal: widget openDialogWithSpec window.
+
+ (widget value contents asString = '')
+ ifTrue: [ self userChoiceName: anElement ]
+ ifFalse: [
+ self rename: (self tmpDirectory: anElement) to: widget value contents asString.
+ self move: (widget value contents asString, '.image') from: (self tmpDirectory: anElement) to: self imageDirectory pathString.
+ self move: (widget value contents asString, '.changes') from: (self tmpDirectory: anElement) to: self imageDirectory pathString.
+ ^ widget value contents asString
+ ]
+
\ No newline at end of file


message:
Refactor a bit.
Fix bug

diff:
@@ -2,4 +2,4 @@ instance creation
category: aString
"Default directory where a .xml should be there"
^ self category: aString
- url: 'https://gitorious.org/pharo-build/pharo-build/blobs/raw/master/pharo-templates/template.xml'
\ No newline at end of file
+ url: 'http://files.pharo.org/templates/template.xml'
\ No newline at end of file

diff:
items creation
fetchOnlineImage
- self listElement: OrderedCollection new.
self listElement addAll: (self fetchOnlineImage: self url)
\ No newline at end of file

diff:
+initialize-release
+initialize
+ super initialize.
+ self listElement: OrderedCollection new.
\ No newline at end of file

diff:
accessing
listElement
+
^ listElement
\ No newline at end of file

diff:
@@ -10,7 +10,7 @@ userChoiceName: anElement
World openModal: widget openDialogWithSpec window.
(widget value contents asString = '')
- ifTrue: [ self userChoiceName: anElement ]
+ ifTrue: [ ^ self userChoiceName: anElement ]
ifFalse: [
self rename: (self tmpDirectory: anElement) to: widget value contents asString.
self move: (widget value contents asString, '.image') from: (self tmpDirectory: anElement) to: self imageDirectory pathString.

diff:
-as yet unclassified
-category: aString url: anURL
- ^ self new
- url: anURL;
- yourself;
- category: aString;
- yourself;
- fetchOnlineImage.
\ No newline at end of file

original_message:
Refactor a bit.
Fix bug


message: fix an error with refresh

diff:
instance creation
initialize
^ self category: 'Default'
- url: 'https://gitorious.org/pharo-build/pharo-build/blobs/raw/master/pharo-templates/template.xml'
\ No newline at end of file
+ url: 'http://files.pharo.org/templates/template.xml'
\ No newline at end of file

diff:
items creation
fetchOnlineImage: anURL
+
| xmlDoc list |
xmlDoc := XMLDOMParser parseDocumentFrom: (anURL asUrl retrieveContents).
list := OrderedCollection new.

diff:
items creation
fetchOnlineImage
- self listElement addAll: (self fetchOnlineImage: self url)
\ No newline at end of file
+ self listElement: (self fetchOnlineImage: self url)
\ No newline at end of file

diff:
initialize-release
initialize
super initialize.
\ No newline at end of file
- self listElement: OrderedCollection new.
\ No newline at end of file

diff:
accessing
listElement: anObject
+
listElement := anObject
\ No newline at end of file

original_message: fix an error with refresh


message: fix stuff, quit image after load another one

diff:
-instance creation
-initialize
- ^ self category: 'Default'
- url: 'http://files.pharo.org/templates/template.xml'
\ No newline at end of file

diff:
-initialize-release
-initialize
- super initialize.
\ No newline at end of file

diff:
-runnnig
+items creation
move: aStringFile from: aStringOrigninalDirectory to: aStringDirectory
OSProcess command:

diff:
-runnnig
+items creation
move: aStringFile to: aStringDirectory
| aString |

diff:
+runnnig
+quit
+ Smalltalk snapshot: false andQuit: true.
\ No newline at end of file

diff:
-runnnig
+path-directory
rename: aStringDirectory to: aString
OSProcess command:

diff:
@@ -8,3 +8,4 @@ run: aString
'/',
aString,
'.image').
+ self quit.
\ No newline at end of file

diff:
-runnnig
+path-directory
tmpDirectory: anElement
^ (self imageDirectory / anElement name asString ) pathString
\ No newline at end of file

diff:
-as yet unclassified
-initialize
- ^ self category: 'Existing'
\ No newline at end of file

diff:
-as yet unclassified
+items creation
buildList: aString
| list |

diff:
-as yet unclassified
+items creation
fetchOnlineImage: anURL
| list |

diff:
-as yet unclassified
+runnnig
loadAndRun: anElement
self run: anElement.
\ No newline at end of file

diff:
-as yet unclassified
+runnnig
run: anElement
OSProcess command:
(Smalltalk vm fullPath asString,
' ',
anElement url asString).
+ self quit.
\ No newline at end of file


message: add parsing .xml in /hoe/user/.config/launcher/templates

diff:
"classinstvars" : [ ],
"pools" : [ ],
"classvars" : [ ],
- "instvars" : [
- "listElement",
- "url"
- ],
+ "instvars" : [ ],
"name" : "PharoLauncherModelExistingImage",
"type" : "normal"
}
\ No newline at end of file

diff:
+A PharoLauncherModelLocalTemplate is a specific model to fetch if templates exists in the (default directory)/templates/
\ No newline at end of file

diff:
+as yet unclassified
+category: aString
+ "Default directory where a .xml should be there, doesn't matter about the string ... for the moment..."
+ ^ self category: aString
+ url: self imageDirectory / 'templates'
\ No newline at end of file

diff:
+as yet unclassified
+buildList: aString
+ ^ self buildList
\ No newline at end of file

diff:
+as yet unclassified
+buildList
+ | list |
+ list := OrderedCollection new.
+ self listElement do:[ :each |
+ list add: (each name) asString ].
+ ^list sorted: [:a :b | a name < b name].
\ No newline at end of file

diff:
+as yet unclassified
+fetchOnlineImage: aFileName
+
+ | xmlDoc list |
+ xmlDoc := XMLDOMParser parseFileNamed: aFileName.
+ list := OrderedCollection new.
+ xmlDoc allElementsNamed: #template do: [:each |
+ list add: (PharoLauncherElement
+ name: (each nodes elementAt: #name) nodes first asString
+ url: (each nodes elementAt: #url) nodes first asString
+ category: (each nodes elementAt: #category) nodes first asString )].
+ ^ list
\ No newline at end of file

diff:
+as yet unclassified
+fetchOnlineImage
+
+ self listElement: OrderedCollection new.
+ (FileList new directory: self url) fileList do:[ :each |
+ self listElement add: (self fetchOnlineImage: each)].
\ No newline at end of file

diff:
+{
+ "commentStamp" : "<historical>",
+ "super" : "PharoLauncherModel",
+ "category" : "Pharo-Launcher-Core",
+ "classinstvars" : [ ],
+ "pools" : [ ],
+ "classvars" : [ ],
+ "instvars" : [ ],
+ "name" : "PharoLauncherModelLocalTemplate",
+ "type" : "normal"
+}
\ No newline at end of file


message: add a new tab named personnal

diff:
as yet unclassified
open
- | defaultTab webTab otherTab existingTab associations tc |
+ | defaultTab webTab otherTab existingTab associations tc personnalTab |
defaultTab := PharoLauncherTabUI open: (PharoLauncherModel category: 'Default') selectCategory: 'Default'.
webTab := PharoLauncherTabUI open: (PharoLauncherModel category: 'Web') selectCategory: 'Web'.
otherTab := PharoLauncherTabUI open: (PharoLauncherModel category: 'Other') selectCategory: 'Other'.
existingTab := PharoLauncherTabUI open: (PharoLauncherModelExistingImage category: 'Existing') selectCategory: 'Existing'.
+ personnalTab := PharoLauncherTabUI open: (PharoLauncherModelLocalTemplate category: 'Personnal') selectCategory: 'Personnal'.
- associations := {'Default' -> defaultTab. 'Web'-> webTab. 'Other'-> otherTab. 'Existing'-> existingTab}.
+ associations := {'Default' -> defaultTab. 'Web'-> webTab. 'Other'-> otherTab. 'Existing'-> existingTab. 'Personnal'-> personnalTab}.
tc := self associations: associations.
tc open
\ No newline at end of file

original_message: add a new tab named personnal


message: fix a bug in PharoLauncherModel (asString -> string)

diff:
items creation
buildList: aString
| list |
+ self halt.
list := OrderedCollection new.
self listElement do:[ :each |
(each category asString = aString)
ifTrue: [ list add: (each name) asString ] ].
+ self halt.
^list sorted: [:a :b | a name < b name].
\ No newline at end of file

diff:
items creation
buildList
-
+ self halt.
^ self buildList: self category
\ No newline at end of file

diff:
@@ -6,7 +6,7 @@ fetchOnlineImage: anURL
list := OrderedCollection new.
xmlDoc allElementsNamed: #template do: [:each |
list add: (PharoLauncherElement
- name: (each nodes elementAt: #name) nodes first asString
- url: (each nodes elementAt: #url) nodes first asString
- category: (each nodes elementAt: #category) nodes first asString )].
+ name: (each nodes elementAt: #name) nodes first string
+ url: (each nodes elementAt: #url) nodes first string
+ category: (each nodes elementAt: #category) nodes first string )].
^ list
\ No newline at end of file


message: After image creation completed, status bar continued to say 'Downloading...'.  Added #showNumberOfErrors to end of #createImageFromTemplate.

diff:
@@ -14,5 +14,5 @@ createImageFromTemplate
UIManager default alert: err longDescription.
^ self ]].
-
+ self showNumberOfErrors.
self updateImages.
\ No newline at end of file


message:
After 'creating an image' from a network repository, the 'Local' templates are not updated. Fixed by adding class PhLCaceLocalTemplateGroup, method #refreshLocal, and related refactoring.
(https://pharo.fogbugz.com/f/cases/11614)

diff:
+action
+refreshLocal
+ self refresh
\ No newline at end of file

diff:
+action
+reset
+ "Request for showing up to date information *without* refreshing groups that would take time to refresh."
+ "So its okay to refresh cache of local template groups."
+ self refresh.
\ No newline at end of file

diff:
+{
+ "commentStamp" : "",
+ "super" : "PhLCacheTemplateGroup",
+ "category" : "Pharo-Launcher-Core",
+ "classinstvars" : [ ],
+ "pools" : [ ],
+ "classvars" : [ ],
+ "instvars" : [ ],
+ "name" : "PhLCacheLocalTemplateGroup",
+ "type" : "normal"
+}
\ No newline at end of file

diff:
+action
+refreshLocal
+"For network template group cache, do nothing. Overriden by subclass for local template group cache"
\ No newline at end of file

diff:
action
reset
"Request for showing up to date information *without* refreshing groups that would take time to refresh."
-
- delegate isLocal
- ifTrue: [ self refresh ]
- ifFalse: [
- "We replace the content by an indication that a full refresh is necessary"
- templatesAndGroups := Array with: PhLEmptyError new ]
\ No newline at end of file
+ "For network cache we replace the content by an indication that a full refresh is necessary"
+ templatesAndGroups := Array with: PhLEmptyError new.
\ No newline at end of file

diff:
private
addGroup: aGroup
- | cache |
- cache := PhLCacheTemplateGroup group: aGroup. "wrap all groups inside a cache."
- aGroup isLocal "always put the local groups before the rest for rapid access"
- ifTrue: [ groupCaches addFirst: cache ]
- ifFalse: [ groupCaches addLast: cache ]
\ No newline at end of file
+ "Always put the local groups before the rest for rapid access"
+ aGroup isLocal
+ ifTrue:
+ [ groupCaches addFirst: (PhLCacheLocalTemplateGroup group: aGroup)
+ ]
+ ifFalse:
+ [ groupCaches addLast: (PhLCacheTemplateGroup group: aGroup)
+ ]
\ No newline at end of file

diff:
+action
+refreshLocal
+ groupCaches do: #refreshLocal
\ No newline at end of file


message:
After 'creating an image' from a network repository, the 'Local' templates are not updated. Fixed by adding method #updateLocalTemplates, and appending it to the end of #createImageFromTemplate.
Requires Pharo-Launcher-Core-BenComan.70.
(https://pharo.fogbugz.com/f/cases/11614)

diff:
@@ -16,3 +16,4 @@ createImageFromTemplate
self showNumberOfErrors.
self updateImages.
+ self updateLocalTemplates.
\ No newline at end of file

diff:
+updating
+updateLocalTemplates
+ templateRepository refreshLocal.
+ templatesModel roots: templateRepository groups.
\ No newline at end of file


diff:
+action
+displayProgress: label during: workBlock
+ UIManager default informUserDuring:
+ [ :bar1|
+ bar1 label: label.
+ UIManager default informUserDuring:
+ [ :bar2|
+ [workBlock value] on: HTTPProgress do:
+ [ : progress |
+ bar2 label: progress printString.
+ progress resume.
+ ].
+ ].
+ ].
+
\ No newline at end of file

diff:
accessing
templatesAndGroups
| content matcher templates |
- content := PhLDownloadManager default downloadToPositionableStream: url mimeType: ZnMimeType textHtml.
+ PhLDownloadManager displayProgress: 'Refreshing ''' , self name , ''' templates' during:
+ [ content := PhLDownloadManager default downloadToPositionableStream: url mimeType: ZnMimeType textHtml.
+ ].
matcher := RxMatcher for: self regex.
templates := OrderedCollection new.
[ matcher searchStream: content ]
@@ -9,3 +11,4 @@ templatesAndGroups
createTemplateFromHref: (matcher subexpression: self subexpression)
addingTo: templates ].
^ templates
+
\ No newline at end of file

diff:
private
jobs
"Get the JSON array from the server"
- | queryUrl query |
+
+ | queryUrl query result |
"The query is detailed enough to get all the information we want with just one request. If you update this, make sure we always have the 'relativePath' field for each artifact."
query := 'jobs[name,url,lastSuccessfulBuild[runs[artifacts[relativePath],fullDisplayName,url],artifacts[relativePath]]]'.
queryUrl := url / 'api' / 'json' ? ('tree' -> query).
- ^ Json readFrom: (PhLDownloadManager default downloadToPositionableStream: queryUrl mimeType: ZnMimeType applicationJson)
\ No newline at end of file
+ PhLDownloadManager
+ displayProgress: 'Refreshing ''' , self name , ''' templates'
+ during: [
+ result := Json
+ readFrom: (PhLDownloadManager default downloadToPositionableStream: queryUrl mimeType: ZnMimeType applicationJson) ].
+ ^ result
\ No newline at end of file

original_message:
https://pharo.fogbugz.com/default.asp?11770
Provide progress bars for network Templates refresh.


message: Remove previously missed breakpoint.

diff:
template action
createImageFromTemplate
| name |
- self halt.
name := imageGroup makeUniqueImageName: self selectedTemplate name.
name := UIManager default request: 'Image name?' initialAnswer: name.
name ifNil: [ ^self ].

original_message: Remove previously missed breakpoint.


diff:
action
launch
"Use the VM that launched the current image to launch me"
- | cdCommand vmCommand |
- cdCommand := 'cd ' , file parent pathString.
- vmCommand := Smalltalk vm fullPath asString , ' ' , file basename.
- OSProcess command: cdCommand , ' && ' , vmCommand
\ No newline at end of file
+ | vmCommand imagePath |
+ vmCommand := Smalltalk vm fullPath asString .
+ imagePath := (file parent / file basename) pathString .
+ OSProcess command: vmCommand , ' ' , imagePath.
\ No newline at end of file

original_message:
https://pharo.fogbugz.com/default.asp?11616
Launching fails in Windows resolved. Changed PhLImage>>Launch from 'changing directory before using a relative pathname for target image' to 'using the absolute pathname for target image'.  Current-working-directory of target image should be unaffected.


diff:
initialize-release
initializeTemplatesAndGroups
- "Set the default value of the templatesAndGroups instance variable. Answer something meaninful only if this does not require time to compute."
+ "Set the default value of the simplifiedTemplatesAndGroups instance variable. Answer something meaninful only if this does not require time to compute."
delegate isLocal
ifTrue: [ self refresh ]
ifFalse: [
"We replace the content by an indication that a full refresh is necessary"
- templatesAndGroups := Array with: PhLEmptyError new ]
\ No newline at end of file
+ simplifiedTemplatesAndGroups := Array with: PhLEmptyError new ]
\ No newline at end of file

diff:
action
refresh
"Request for fetching the content of my template group (and that can take time if my group downloads content from the web)"
- templatesAndGroups := nil
\ No newline at end of file
+ simplifiedTemplatesAndGroups := nil
\ No newline at end of file

diff:
+accessing
+simplifiedTemplatesAndGroups
+ ^ simplifiedTemplatesAndGroups
+ ifNil: [
+ "#simplifiedTemplatesAndGroups will typically use the network and may fail. If the network is unreliable, it is better to save the error and not call #templateAndGroups a thousand times. Let the user interface decide when to try again (by calling #reset)."
+ simplifiedTemplatesAndGroups := [ delegate simplifiedTemplatesAndGroups ]
+ on: PhLError
+ do: [ :err | Array with: err ] ]
\ No newline at end of file

diff:
-accessing
-templatesAndGroups
- ^ templatesAndGroups
- ifNil: [
- "#templatesAndGroups will typically use the network and may fail. If the network is unreliable, it is better to save the error and not call #templateAndGroups a thousand times. Let the user interface decide when to try again (by calling #reset)."
- templatesAndGroups := [ delegate templatesAndGroups ]
- on: PhLError
- do: [ :err | Array with: err ] ]
\ No newline at end of file

diff:
"classvars" : [ ],
"instvars" : [
"delegate",
- "templatesAndGroups"
+ "simplifiedTemplatesAndGroups"
],
"name" : "PhLCacheTemplateGroup",
"type" : "normal"

original_message:
https://pharo.fogbugz.com/default.asp?11837
Performance issue with Jenkins templatesAndGroups

PhLCacheTemplateGroup needed to cache simplifiedTemplatesAndGroups rather templatesAndGroups.


message: take care of cancel in dialogs

diff:
@@ -3,5 +3,6 @@ copyImage
| image newName |
image := self selectedImage.
newName := UIManager default request: 'New name'.
+ newName ifNil: [ ^self ].
imageGroup copyImage: image to: newName.
self updateImages
\ No newline at end of file

diff:
@@ -3,5 +3,6 @@ renameImage
| image newName |
image := self selectedImage.
newName := UIManager default request: 'New name'.
+ newName ifNil: [ ^self ].
imageGroup renameImage: image to: newName.
self updateImages
\ No newline at end of file

original_message: take care of cancel in dialogs


message: do not copy/rename to existing images

diff:
+private
+allImages
+
+ ^imageGroup images
\ No newline at end of file

diff:
@@ -2,7 +2,8 @@ image action
copyImage
| image newName |
image := self selectedImage.
- newName := UIManager default request: 'New name'.
+ newName := UIManager default request: 'New name' initialAnswer: image name.
newName ifNil: [ ^self ].
+ (self hasAlreadyImageWithName: newName) ifTrue: [ ^UIManager default inform: 'There is already an image with this name' ].
imageGroup copyImage: image to: newName.
self updateImages
\ No newline at end of file

diff:
+private
+hasAlreadyImageWithName: newName
+ "Return true if we already have an image with the given name"
+
+ ^(self allImages select: [:each | each name = newName ]) notEmpty
\ No newline at end of file

diff:
@@ -2,7 +2,8 @@ image action
renameImage
| image newName |
image := self selectedImage.
- newName := UIManager default request: 'New name'.
+ newName := UIManager default request: 'New name' initialAnswer: image name.
newName ifNil: [ ^self ].
+ (self hasAlreadyImageWithName: newName) ifTrue: [ ^UIManager default inform: 'There is already an image with this name' ].
imageGroup renameImage: image to: newName.
self updateImages
\ No newline at end of file

diff:
updating
updateImages
| images |
- images := imageGroup images sorted: [ :im1 :im2 | im1 name < im2 name ].
+ images := self allImages sorted: [ :im1 :im2 | im1 name < im2 name ].
imagesModel roots: images
\ No newline at end of file

original_message: do not copy/rename to existing images


message: make refresh button smaller so it does not start to repeat the icon again

diff:
@@ -3,5 +3,5 @@ layout
<spec: #default>
^ SpecLayout composed
add: #label;
- add: #refreshButton origin: 1@0 corner: 1@1 offsetOrigin: -25@0 offsetCorner: 0@0;
+ add: #refreshButton origin: 1@0 corner: 1@1 offsetOrigin: -23@0 offsetCorner: 0@0;
yourself
\ No newline at end of file


message: add separator between "launch" and image/file handling

diff:
menu
imagesMenu: menu
- menu add: 'Launch' target: self selector: #launchImage.
+ menu add: 'Launch image' target: self selector: #launchImage.
+ menu addLine.
menu add: 'Rename' target: self selector: #renameImage.
menu add: 'Copy' target: self selector: #copyImage.
menu add: 'Delete' target: self selector: #deleteImage
\ No newline at end of file


message: wrap cmd call in #launch with $" to also directories with spaces like c:\documents and settings\myuser

diff:
@@ -4,4 +4,4 @@ launch
| vmCommand imagePath |
vmCommand := Smalltalk vm fullPath asString .
imagePath := file pathString.
- OSProcess command: vmCommand , ' ' , imagePath.
\ No newline at end of file
+ OSProcess command: vmCommand , ' "' , imagePath, '"'.
\ No newline at end of file


message: Cleanup. Now that has been integrated remove workaround in #deleteImages introduced with .

diff:
@@ -3,10 +3,7 @@ deleteImages
| images imageNames confirmMessage |
images := self selectedImages.
imageNames := images collect: [ :image | image name ].
- self flag: 'Workaround required until https://pharo.fogbugz.com/default.asp?11915 integrated'.
- imageNames size = 1
- ifTrue: [ confirmMessage := 'Are you sure you want to delete ' , imageNames first asString ]
- ifFalse: [ confirmMessage := 'Are you sure you want to delete ' , (imageNames joinUsing: ', ' last: ' and ') ].
+ confirmMessage := 'Are you sure you want to delete ' , (imageNames joinUsing: ', ' last: ' and ').
(UIManager default confirm: (confirmMessage withNoLineLongerThan: 60) , '?')
ifFalse: [ ^ self ].
imageGroup deleteImages: images.

original_message: Cleanup. Now that  https://pharo.fogbugz.com/default.asp?11915 has been integrated remove workaround in #deleteImages introduced with https://pharo.fogbugz.com/default.asp?11888 .


diff:
initialization
initializeWidgets
- self instantiateModels: #(labelModel LabelModel)
\ No newline at end of file
+ labelModel := LabelModel new.
\ No newline at end of file

diff:
initialization
initializeWidgets
- self instantiateModels: #(#label #LabelModel #refreshButton #ButtonModel).
- label text: ''.
- refreshButton
+ (label := LabelModel)
+ text: ''.
+
+ (refreshButton := ButtonModel)
icon: Smalltalk ui icons refreshIcon;
action: refreshAction;
- state: false
\ No newline at end of file
+ state: false.
\ No newline at end of file

diff:
initialization
initializeWidgets
- self instantiateModels: #(titlebar PhLTitlebarModel tree TreeModel).
-
\ No newline at end of file
+ titlebar := PhLTitlebarModel.
+ tree := TreeModel.
\ No newline at end of file

diff:
initialization
initializeWidgets
- self
- instantiateModels:
- #(#imagesModel #PhLTitledTreeModel #templatesModel #PhLTitledTreeModel #statusbarModel #PhLStatusbar).
- imagesModel
+
+ (imagesModel := PhLTitledTreeModel new)
title: 'Existing images' translated;
refreshAction: [ self updateImages ];
displayBlock: [ :image | image name ];
doubleClick: [ self launchImage ];
menu: [ :menu | self imagesMenu: menu ];
makeMultiSelect.
- templatesModel
+
+ (templatesModel := PhLTitledTreeModel new)
title: 'Templates' translated;
refreshAction: [ self refreshTemplateGroups ];
displayBlock: [ :group | group name ];
childrenBlock: [ :group | group children: self ];
menu: [ :menu | self templatesMenu: menu ].
+
+ statusbarModel := PhLStatusbar new.
+
self updateImages.
templatesModel roots: templateRepository groups.
\ No newline at end of file

original_message:
Replace Spec #instantiateModels: with direct ivar assignment per suggestion
http://forum.world.st/Spec-Lists-setting-the-selection-tp4713776p4713842.html


message: Quick fix, i added missing "new"

diff:
initialization
initializeWidgets
- (label := LabelModel)
+ (label := LabelModel new)
text: ''.
- (refreshButton := ButtonModel)
+ (refreshButton := ButtonModel new)
icon: Smalltalk ui icons refreshIcon;
action: refreshAction;
state: false.
\ No newline at end of file

diff:
initialization
initializeWidgets
- titlebar := PhLTitlebarModel.
- tree := TreeModel.
\ No newline at end of file
+ titlebar := PhLTitlebarModel new.
+ tree := TreeModel new.
\ No newline at end of file

original_message: Quick fix, i added missing "new"


change_type: RENAME

diff:
specs
-layout
+specLayout
<spec: #default>
^ SpecLayout composed
add: #labelModel;

diff:
specs
-layout
+specLayout
<spec: #default>
^ SpecLayout composed
add: #label;

diff:
specs
-layout
+specLayout
<spec: #default>
^ SpecLayout composed
newColumn: [ :col |

diff:
@@ -2,3 +2,4 @@ initialization
initializeWidgets
titlebar := PhLTitlebarModel new.
tree := TreeModel new.
+
\ No newline at end of file

diff:
initialization
initializeWidgets
-
(imagesModel := PhLTitledTreeModel new)
title: 'Existing images' translated;
refreshAction: [ self updateImages ];

original_message:
Changed remaining class-side #layout to #specLayout.

Modified #initializeWidgets methods using #instantiateModels: to direct ivar assignment per http://forum.world.st/Spec-Lists-setting-the-selection-tp4713776p4713842.html


message: Pharo-Launcher-UI-BenComan.64 should have been done like this.

diff:
initialization
initializeWidgets
- labelModel := LabelModel new.
+ labelModel := self newLabel.

diff:
initialization
initializeWidgets
- (label := LabelModel new)
+ (label := self newLabel)
text: ''.
- (refreshButton := ButtonModel new)
+ (refreshButton := self newButton)
icon: Smalltalk ui icons refreshIcon;
action: refreshAction;
state: false.
\ No newline at end of file

diff:
initialization
initializeWidgets
- titlebar := PhLTitlebarModel new.
- tree := TreeModel new.
+ titlebar := self newTItleBar.
+ tree := self newTree.
\ No newline at end of file

diff:
+widgets
+newTitleBar
+ self instantiate: PhLTitlebarModel.
\ No newline at end of file

diff:
initialization
initializeWidgets
- (imagesModel := PhLTitledTreeModel new)
+ (imagesModel := self newTitledTree)
title: 'Existing images' translated;
refreshAction: [ self updateImages ];
displayBlock: [ :image | image name ];
@@ -8,14 +8,14 @@ initializeWidgets
menu: [ :menu | self imagesMenu: menu ];
makeMultiSelect.
- (templatesModel := PhLTitledTreeModel new)
+ (templatesModel := self newTitledTree)
title: 'Templates' translated;
refreshAction: [ self refreshTemplateGroups ];
displayBlock: [ :group | group name ];
childrenBlock: [ :group | group children: self ];
menu: [ :menu | self templatesMenu: menu ].
- statusbarModel := PhLStatusbar new.
+ statusbarModel := self newStatusBar.
self updateImages.
templatesModel roots: templateRepository groups.
\ No newline at end of file

diff:
+widgets
+newStatusBar
+ self instantiate: PhLStatusbar.
+

diff:
+widgets
+newTitledTree
+ self instantiate: PhLTitledTreeModel.
\ No newline at end of file


message: Fixed some simple errors.

diff:
initialization
initializeWidgets
- titlebar := self newTItleBar.
+ titlebar := self newTitleBar.
tree := self newTree.
\ No newline at end of file

diff:
widgets
newTitleBar
- self instantiate: PhLTitlebarModel.
\ No newline at end of file
+ ^ self instantiate: PhLTitlebarModel.
\ No newline at end of file

diff:
widgets
newStatusBar
- self instantiate: PhLStatusbar.
+ ^ self instantiate: PhLStatusbar.

diff:
widgets
newTitledTree
- self instantiate: PhLTitledTreeModel.
\ No newline at end of file
+ ^ self instantiate: PhLTitledTreeModel.
\ No newline at end of file

original_message: Fixed some simple errors.