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

OpenFileDialog at a specific target #2301

Closed
JeoffreyGi opened this issue Jun 21, 2022 · 19 comments
Closed

OpenFileDialog at a specific target #2301

JeoffreyGi opened this issue Jun 21, 2022 · 19 comments

Comments

@JeoffreyGi
Copy link
Contributor

Hello,
Today, you can create an Dialog box to choose a file with the Python script. You can launch it in the FileUtil class :

new OpenFileDialog().promptForFile(window, "Open File", null, null);

But, you can't choose the first location of the OpenFileDialog.
To work, tt seem to be :
new OpenFileDialog().promptForFile(window, "Open File", "My Path", null);

Whereas, this is not implements.
Maybe we could add this figure.
I can make it.

But if we can do it without change the Java code, i would be glad to know how.
I want to launch a script which settle parameters, and one is the file to use.

@kasemir
Copy link
Collaborator

kasemir commented Jun 21, 2022

You're right, the current set of FileUtil.openFileDialog,

public static String openFileDialog(boolean inWorkspace)
as well as FileUtil.openSaveDialog
public static String saveFileDialog(boolean inWorkspace)
only take a boolean inWorkspace which is there for compatibility with the older Eclipse version that had a 'workspace' and no longer has any meaning.

Feel free to create a pull request which adds for example this:

/** Open a file save dialog.
 * @param path_to_file Path to existing file, used to set the initial directory on which the file dialog is opened.  May be null
 * @return Full file path. Or null if dialog is cancelled.
 */
public static String saveFileDialog(String path_to_file)

@JeoffreyGi
Copy link
Contributor Author

I've solved the issue but i can't push my branch :
git push -u upstream dialogBoxFileUtil
Error : Permission to ControlSystemStudio/phoebus.git denied to JeoffreyGi.
After looking in the project, i didn't find the solution.
I'm new to GitHub so i don't have know how to get the right.

@kasemir
Copy link
Collaborator

kasemir commented Jun 23, 2022

I've solved the issue

Excellent!

but i can't push my branch :

See github intro https://docs.github.com/en/get-started/quickstart/fork-a-repo

@JeoffreyGi
Copy link
Contributor Author

Thank you, the pull request is ready.

@kasemir
Copy link
Collaborator

kasemir commented Jun 23, 2022

Closed with #2306, thanks!

@kasemir kasemir closed this as completed Jun 23, 2022
@georgweiss georgweiss reopened this Aug 12, 2022
@georgweiss
Copy link
Collaborator

The code change for this issue has broken a feature in the FileBrowser: if user right clicks on a directory node to create a new display or data browser plot, the file chooser dialog is not initialized correctly with the directory on which user has clicked. In other words: that target for the dialog is wrong.

@JeoffreyGi , can you please take a look?

@JeoffreyGi
Copy link
Contributor Author

I apologies but that is, i don't understand the probleme, and can't see the difference between before and after my commit, or it's not me.
In the FileBrowser i have not issue to créate new plot, or new display.
But it may possible that, with my changes, this issue could be correct.

Using openSaveFileDialog(String) to replace openFileDialog(Boolean) when you select "new display"
My modification was to alter the pre-settings for the dialog, in that case i didn't touch the event in the dialog.

I only checked if i were the generator of the issue and didn't check the code directly

@georgweiss
Copy link
Collaborator

Issue is that the target directory in the file dialog of the new display/plot file is wrong. It should be the same as the directory as where user has right-clicked to launch the context menu.

With this change the dialog is not initialized with a directory, so it defaults to something. This default on Mac is the user's Desktop directory, on CentOS Linux it is the user's home directory.

@JeoffreyGi
Copy link
Contributor Author

Yes that issue exist before too. I checked, and that was the case too.
the default is the preference setting set by the user.
So without pref you go to default setting from computer.
I add the possibility to open the dialog exactly where you want it

Before it wasn't worked either.

@georgweiss
Copy link
Collaborator

I disagree. This worked fine before: if user right clicks on any directory, the new display/plot file was created in that directory.

@JeoffreyGi
Copy link
Contributor Author

Come on, try this checkout, it's before my changes
3567e89
I agree you can complain to other changes but check it before accuse someone.

@georgweiss
Copy link
Collaborator

In your commit d892353 the SaveAsDialog.doPropmtForFile() method reads:

 private File doPromptForFile(final Window window, final String title, File file, final ExtensionFilter[] filters)
    {
        File initial_directory;
        final FileChooser dialog = new FileChooser();
        dialog.setTitle(title);
        
        if (!Preferences.default_save_path.isEmpty()){
            initial_directory = new File(Preferences.default_save_path);
            dialog.setInitialDirectory(initial_directory);
        }
        
        if (file != null)
        {
            // Dialog will fail if the directory does not exist
        	if(file.exists()) 
        	{
        		
        		if(file.isDirectory()) 
        		{
        			dialog.setInitialDirectory(file);
        		}
        		else if (null != file.getParentFile() && file.getParentFile().exists()) 
        		{
                    dialog.setInitialDirectory(file.getParentFile());
                    dialog.setInitialFileName(file.getName());
        			
        		}
        	}
            
        }
        
        if (filters != null)
            dialog.getExtensionFilters().addAll(filters);
        
        return doExecuteDialog(window, dialog);
    }
    

When called from the context menu handler in the case I describe, file can be a File reference to a non-existing file. So the check if file exists returns false, and setInitialDirectory is never called.

I apologize if this change is not from you, but this is what I conclude from the git commit history.

@JeoffreyGi
Copy link
Contributor Author

First of all, in your case, if the file does not exist, how did you right click on it ? Because I recall, a file in java is just a path and could be whatever. So if the file don't exist the dialog can't open on it.

And, one more thing you should know, before, there was no fonction who call promptForFile with File != null. It just doesn't existed before. SO YOU COULDN'T OPEN A DIALOG AT A SPECIFIC PLACE, THAT'S WHY I MADE THIS TICKET.

You conclude it's from me without even test the checkout, just checkout, compile and try. If you are a developper, you know how to test no ?

@georgweiss
Copy link
Collaborator

The right-click in the FileBrowser is invoked on an existing directory. However, the context menu handler constructs a Java File object with the default name "new_display", using the directory the user has clicked on in the FileBrowser. This File object is used in the call to doPromptForFile. If a user has previously saved a file with this name in the directory, then the file exists. If there is no file named new_display (or actually new_display.bob, the code adds the extension) in the directory, then the File object references a non-existing file. This is a perfectly valid use case.

So: doPromptForFile may be called with a reference to a non-existing file. And this was handled before your change. For the sake of clarity: I reverted back to the previous version of SaveAdDialog (commit 6e9eddb) and then it works a I expect it to work.

@JeoffreyGi
Copy link
Contributor Author

So you return 2 years earlier, to check if i made a mistake there is 2 mounth...
Do you realize the problem ?
Why check 2 years earlier than before my modification ?
Furthermore, it don't work BEFORE my work as well.

@kasemir
Copy link
Collaborator

kasemir commented Aug 12, 2022

Looks like it's Friday with everybody ready to start the weekend.
I don't see anybody directly at fault here, we're all just trying to do incremental improvements, and this is just a bug. A nuisance, but won't keep any end user from getting their job done. (Well, OK, the directory structure of a control system installation can be large and some end users won't know where they are if the file dialog pops up just anywhere).
The updated FileUtil might not be directly related, but the file dialog opened from the file browser certainly has an issue.
At some time, it was possible to right click on a folder, or on any exiting file inside a folder, and the "New Display", "New Plot", "New Folder" actions would then start there.
Looks like we already see where the parent folder info is getting lost, so let's fix it.

@georgweiss
Copy link
Collaborator

I compare to a 2 year old commit since that is the version preceding your commit of SaveAsDialog.

If I checkout the commit you propose (3567e89) the FileBrowser feature we are discussing works fine, just as expected.

@shroffk
Copy link
Member

shroffk commented Aug 12, 2022

I added an additional check which should handle the case of new (as yet non existent) file and get the existing parent folder.
This should address the initialization issue with regards to context menu actions.

@georgweiss
Copy link
Collaborator

georgweiss commented Aug 15, 2022

@shroffk 's pull request #2355 verified to solve the issue.

shroffk added a commit that referenced this issue Aug 15, 2022
#2301 use parent folder when creating a new (non existent file)
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

No branches or pull requests

4 participants