eclab / edisyn

Synthesizer Patch Editor
Apache License 2.0
550 stars 51 forks source link

Select directory problem on macos #41

Closed samstaton closed 3 years ago

samstaton commented 3 years ago

In Batch Download, "select directory" is causing a null pointer exception on macos 10.14.6. So batch download doesn't work. Fix seems to be:

Synth.java
6960c6960
<                 return new File(fd.getFile());
---
>                 return new File(fd.getDirectory());
SeanLuke commented 3 years ago

Unfortunately this doesn't work. getDirectory() choose the parent of the chosen directory. Since getFile() is returning null, this is very bad news -- we'll have to go back to JFileChooser for making new directories. :-(

samstaton commented 3 years ago

Ah, it is working for me, but it may be specific to different macos versions.

My experiments suggested that the chooser picks a new file (you seem to have to give a file name when you use it, even though we are wanting to choose a directory, not a file, so I just leave the file name as "Untitled"). And getDirectory() is returning the directory where you chose to put the file, which is the directory that you want. Whereas getFile() is just returning the string of the short file name (e.g. Untitled), and so getParent of that string is null.

eclab commented 3 years ago

This is the case for me too. But unfortunately that's not how the Mac interface is supposed to work; for example, let's say you clicked on a file in an ordinary save panel. That file would then appear in the "Save as:" text box. But here, if you click on a directory (notionally the target "file" of a FileChooser set up on the Mac to select directories) it doesn't appear in the text box -- it just works like how you normally click on directories. :-(

We can't use the "Load" file chooser because the button says "Open".

The only alternatives would be (1) to use the horrific JFileChooser or (2) change the default filename in the Save box to something like "Choose a Directory". I think it'd be very confusing though. Do you have any suggestions for the text for #2? It's gotta be [A] short [B] informs the user that he needs to pick a directory, then press "save" [3] make it clear that he can't type a directory name in the save text box.

On Sep 8, 2021, at 9:14 AM, samstaton @.***> wrote:

Ah, it is working for me, but it may be specific to different macos versions.

My experiments suggested that the chooser picks a new file (you seem to have to give a file name when you use it, even though we are wanting to choose a directory, not a file, so I just leave the file name as "Untitled"). And getDirectory() is returning the directory where you chose to put the file, which is the directory that you want. Whereas getFile() is just returning the string of the short file name (e.g. Untitled), and so getParent of that string is null.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://secure-web.cisco.com/1jL1Hr69KQDP1VXd_lx94zRSnk0w5M-IGMPJPKSFG76eK3iscaVZlNG8984Iln_NjDmdK1oUeHZKRexKsbfFlorsSCHUhPQiReX2_rKTGBFMF0ZSOOkjxgter-8swUxDVmwvvoK_UoNTKOqiDUvCX2vIcD1FnNI-hMCm4MVdEZ_BxaFS9O2ihvd_-mBmXVd3JHaQ4mScOTFmrIUWdyPa_T46ekF-zd0tgUpvKm2Cc4m1hpEJNC-Zp9CWAIgU9TCoAJHL3di453AULgjdB6gkiVmusqYYqWMincMzwyhiK6r4QyOtHaBwqBwORXw2eWYiYiN_pRK0sHHVAeGoJi7fXIIc8pqlP5U_FhdSP44wgbHiUy6iIYaHEFup_xmgW0WBJxbtRMNZpPvwVu7ymVYBrVZ-XYvNcCcBwnQyFWr1FkNNEu-sf8KBW6WVvbeGtBHt1/https%3A%2F%2Fgithub.com%2Feclab%2Fedisyn%2Fissues%2F41%23issuecomment-915228258, or unsubscribe https://secure-web.cisco.com/1-PAFvgydFCmCOog7uMiqdK_wuJVdlJHyTkMp-1dgV7AQ-l3AHzd38NeZ50PmT5z1i8S4jx--up2G_sE1QhUqxc06bDGJIo-cqSLxspQnj-mTzEXJ3Vy8K-l-UXeystjRpIatbPX6OB9lwYNMlzmDSSys-GjznilMDOTbRDDLJgu!%20ABZCDcYL2RgpstbjWtyCaoNo0_E_WgDk-TkOYREvRAEMNgmhNWqY0EAzSXULKg9ESAtZ6-RXBfVy-CuMuITXxYqXjGaRV73fYmlRBZDaQLLi2MtButDAk-KlMKfJPxmutdbdXccU1BR1F1w2a36thwPkLVHi4R_S7wSI1GjafQR1VU7deH3Ra0c8qL0MFCiTy81OdtGDxWP6vzjLgT_n9h9WA5kH8JIRUVonSBofG14YmZGs-_QOCBKRA66bSRD50UtKOHdd_-PVOK3kEKCVh/https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FADAZDVGYLBBTKMTUVF4XKYTUA5OS7ANCNFSM5DUULO4Q. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

samstaton commented 3 years ago

Ah, I tried the "Load" file chooser and I think it works much more intuitively. I see what you mean about "Open", but it is not too bad, since you are genuinely opening the folder in which the patches should be saved. With the "Save" file chooser, I can't select the folder that I want, I have to go into its parent folder and then manually type its name, which is a bit weird, and then I get a warning that it already exists and will be overwritten, which is not really true -- I mean existing files in the folder aren't touched unless there is a name overlap.

(For either of these options I think line 6960 should be return new File(fd.getDirectory(), fd.getFile());.)

eclab commented 3 years ago

On Sep 8, 2021, at 12:22 PM, samstaton @.***> wrote:

Ah, I tried the "Load" file chooser and I think it works much more intuitively. I see what you mean about "Open", but it is not too bad, since you are genuinely opening the folder in which the patches should be saved.

Well the other problem is that you can't create a folder to "open" in the Load file chooser -- you have to select an existing folder. :-( Of course this is the same misfeature that JFileChooser has. .(For either of these options I think line 6960 should be return new File(fd.getDirectory(), fd.getFile());.)

Are you sure? Why not just getFile()?

samstaton commented 3 years ago

Oh, on my macos, I get a "new folder" button in the Load file chooser (bottom left), so it is all working nicely.

For me, fd.getFile() just returns the file name without path, and so I'm getting a null pointer when you later take it's parent. But new File(fd.getDirectory(), fd.getFile()); seems to work fine, no null pointers.

SeanLuke commented 3 years ago

Thanks for the excellent bug report. I've committed code using just FileDialog.LOAD and new File(fd.getDirectory(), fd.getFile()); I'm closing this issue for now, but reopen it if I screwed something up.