Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Conversion to an output file writes to a directory instead of a file #344

Closed
geographika opened this issue May 9, 2023 · 6 comments
Closed

Comments

@geographika
Copy link
Collaborator

This issue is linked to changes in #335
The code below checks if the output parameter exists and if it is a file, however when writing an output this file won't exist, and targetIsFile is always false.

  // Try to define type of target (file or dir).
  let targetIsFile = false;
  const outputExists = existsSync(outputPath);
  if (outputExists) {
    targetIsFile = lstatSync(outputPath).isFile();
  }

This leads to the output being written to subdirectories based on the input e.g.

npm start -- -s qgis -t qgis -o output.sld testdata/point_simple.qml creates output.qml/point_simplepoint.qml

The workflow in #343 highlights this issue.
See for example the output at https://github.com/geographika/geostyler-cli/actions/runs/4925040568/jobs/8798798475#step:8:13

I'm unsure of the best approach to fix this. Would simply checking for an extension of the output parameter to set targetIsFile be enough?

@KaiVolland
Copy link
Contributor

If an output is specified it should always "win". I guess i just missed to test this enough. So if you are able to fix this please go ahead.

@geographika
Copy link
Collaborator Author

@KaiVolland - the issue is the output could be specified as a file or folder. As they might not yet exist I can only think of checking for a file extension in the-o parameter. A folder could contain . though e.g./tmp/09.05.2023/folder
Maybe we could require the specified output root folder to exist?
It would also be good to add a test to #343 for processing a folder. Should I add some test files for this?

@KaiVolland
Copy link
Contributor

We could also check if the -o parameter contains at least one path.sep (/) if not we assume it is a file. And if you want a folder to be create at root level it has to start with ./?

@geographika
Copy link
Collaborator Author

We could also check if the -o parameter contains at least one path.sep (/) if not we assume it is a file. And if you want a folder to be create at root level it has to start with ./?

A common case could be to put a path to a file e.g. /mystyles/output.json', and on Windows a folder could be C:\Temp`. I can't think of a consistent way of telling if a string is a path to a folder or file which doesn't yet exist.

Attempted fix at #345 but it relies on the root folder being created first.

Maybe checking if the source is a folder (which has to exist) would be a better option? If a user put a path to a file in the output parameter then it would simply create a folder with a "file-like" name.

@jansule
Copy link
Contributor

jansule commented May 27, 2024

@geographika is this issue solved?

@geographika
Copy link
Collaborator Author

@jansule - yes fixed in https://github.com/geostyler/geostyler-cli/pull/347/files. Closing.

  // Assume the target is the same as the source
  let targetIsFile = sourceIsFile;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants