search

FashionFreedom / Seamly2D

Open source patternmaking software to democratize fashion.
https://seamly.io
GNU General Public License v3.0
617 stars 111 forks source link

Bug: Improper handling of missing measurements #1212

Open slspencer opened 1 week ago

slspencer commented 1 week ago

When opening a pattern where the designated measurement file can't be found, the user can select another measurement file. If the selected measurement file doesn't contain all of the required measurements, the following error message is displayed. When the user clicks 'OK' the pattern closes.

Capture

This popup message could provide the user more error handling options.

These options could be:

  1. Select a new measurement file.
  2. Open the selected measurement file in SeamlyMe. If selected, ask the user whether to add the missing measurements, Y/N automatically.
  3. Close and return to Seamly2D.

Also, add the path and measurement file name in the error message for clarity.

DSCaskey commented 1 week ago
  1. Select a new measurement file.
  2. Open the selected measurement file in SeamlyMe. If selected, ask the user whether to add the missing measurements, Y/N automatically.
  3. Close and return to Seamly2D.

Doesn't quite work that way. You have to keep in mind that Seamly2D can be running from a command line with no GUI, and you have to account for that. If Seamly is running from the command line and it can't find the measurement file it has to abort - thus the File excpetion. The problem is the loadPattern(), checkPathToMeasurments(), and checkRequiredMeasurements() methods are a mess. Since the gui/nogui modes here are coupled together, even when in gui mode it throws the file exception if there are missing measurements, rather than taking a more graceful course of action. And actually throwing the exception in no GUI mode makes no sense as there is no GUI to see the message box... just need to exit the app.

slspencer commented 1 week ago

per @DSCaskey - checkRequiredMeasurements() throws an exception, but it should only return whether the measurements are valid. The exception should be thrown elsewhere. This workflow should be broken up into smaller, more fine grained functions(), but won't be handled right away.