Skip to content

Changed command structure to fix windows glide bug #15

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 16, 2025
Merged

Changed command structure to fix windows glide bug #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
65f0435
Changed command structure to fix windows glide bug
Daniel-Estoll Jun 16, 2025
9217f4b
Added comment to explain strange syntax
Daniel-Estoll Jun 16, 2025
c7c1c52
Allowed directories to be specified in the command
Daniel-Estoll Jun 16, 2025
2b42d6a
Updated excludeCommand quotations and comment
Daniel-Estoll Jun 16, 2025
2562716
Updated version
Daniel-Estoll Jun 16, 2025
351c826
Added package-lock.json
Daniel-Estoll Jun 16, 2025
dedee00
Merge remote-tracking branch 'origin/develop' into 4504-p3-glide-bug-…
Daniel-Estoll Jun 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "polyapi",
"version": "0.23.25",
"version": "0.23.26",
"description": "Poly is a CLI tool to help create and manage your Poly definitions.",
"license": "MIT",
"repository": {
Expand Down
37 changes: 22 additions & 15 deletions src/deployables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,26 +170,33 @@ export const getAllDeployableFilesWindows = ({
excludeDirs,
}: PolyDeployConfig): string[] => {
// To get the equivalent of grep in Windows we use a combination of `dir` and `findstr`
const includePattern =
includeFilesOrExtensions.length > 0
? includeFilesOrExtensions
.map((f) => (f.includes('.') ? f : `*.${f}`))
.join(' ')
: '*';
const excludePattern = excludeDirs.length > 0 ? excludeDirs.join('|') : '';
const excludePattern = excludeDirs.length > 0
? excludeDirs
.map((f) => `\\${f}`)
.join(' ') : '';
const pattern =
typeNames.length > 0
? typeNames.map((name) => `polyConfig: ${name}`).join('|')
? typeNames.map((name) => `\\<polyConfig: ${name}\\>`).join(' ')
: 'polyConfig';

// Using two regular quotes or two smart quotes throws "The syntax of the command is incorrect".
// For some reason, starting with a regular quote and leaving the end without a quote works.
const excludeCommand = excludePattern
? ` | findstr /V /I "${excludePattern}"`
? ` | findstr /V /I "${excludePattern}`
: '';
const searchCommand = ` | findstr /M /I /F:/ /C:"${pattern}"`;
const searchCommand = ` | findstr /M /I /F:/ ${pattern}`;

let result: string[] = [];
for (const dir of includeDirs) {
const dirCommand = `dir /S /P /B ${includePattern} ${dir}`;
const includePattern =
dir === '.'
? includeFilesOrExtensions
.map((f) => (f.includes('.') ? f : `*.${f}`))
.join(' ')
: includeFilesOrExtensions
.map((f) => (f.includes('.') ? f : `${dir}*.${f}`))
.join(' ');
const dirCommand = `dir ${includePattern} /S /P /B`;
Copy link
Member

@aarongoin aarongoin Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something seems off here. If we don't put the directory we're interested in are we going to be searching the same thing over and over again in this for loop?

Copy link
Contributor Author

@Daniel-Estoll Daniel-Estoll Jun 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

includedDirs only contains one item: ['.']. The command only ever runs once, and it runs in the directory that it is called in. If we actually have a specific directory we can add ${dir} before ${includePattern}, but right now if we just add '.' it will return folders and other things we don't want.

Copy link
Member

@aarongoin aarongoin Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Daniel-Estoll Include dirs happens to only contain one directory right now, but that could change in the future via configuration. Also I'm realizing there's a BUG in getAllDeployableFiles which we should fix. Do you mind taking care of it in your PR here?

This code:

config.includeDirs = config.includeDirs = ['.'];
  config.includeFilesOrExtensions = config.includeFilesOrExtensions = [
    'ts',
    'js',
  ];

should actually be:

config.includeDirs = config.includeDirs || ['.'];
config.includeFilesOrExtensions = config.includeFilesOrExtensions || [
  'ts',
  'js',
];

Copy link
Contributor Author

@Daniel-Estoll Daniel-Estoll Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

const fullCommand = `${dirCommand}${excludeCommand}${searchCommand}`;
try {
const output = shell.exec(fullCommand).toString('utf8');
Expand Down Expand Up @@ -230,13 +237,13 @@ export const getAllDeployableFilesLinux = ({
export const getAllDeployableFiles = (
config: Partial<PolyDeployConfig> = {},
): string[] => {
config.typeNames = config.typeNames = DeployableTypeEntries.map((p) => p[0]);
config.includeDirs = config.includeDirs = ['.'];
config.includeFilesOrExtensions = config.includeFilesOrExtensions = [
config.typeNames = config.typeNames || DeployableTypeEntries.map((p) => p[0]);
config.includeDirs = config.includeDirs || ['.'];
config.includeFilesOrExtensions = config.includeFilesOrExtensions || [
'ts',
'js',
];
config.excludeDirs = config.excludeDirs = [
config.excludeDirs = config.excludeDirs || [
Comment on lines +240 to +246
Copy link
Member

@aarongoin aarongoin Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

'node_modules',
'dist',
'build',
Expand Down