Skip to content

Conversation

@t-b
Copy link
Collaborator

@t-b t-b commented Dec 10, 2025

No description provided.

@t-b t-b self-assigned this Dec 10, 2025
Copilot AI review requested due to automatic review settings December 10, 2025 20:35
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new public API function AB_AddFolder to enable adding folders to the analysis browser programmatically, particularly for use from Python scripts. The change refactors existing button handler code to use this new function and makes the internal helper function AB_AddElementToSourceList static to clarify the API boundary.

Key Changes:

  • Adds new public function AB_AddFolder(string win, WAVE/T folders) for adding folders to the analysis browser
  • Refactors AB_ButtonProc_AddFolder to use the new AB_AddFolder function
  • Makes AB_AddElementToSourceList static to restrict it to internal use only

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +3234 to +3240
string folder

for(folder : folders)
AB_AddElementToSourceList(folder)
endfor

AB_AddExperimentEntries(win, folders)
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The new AB_AddFolder function is missing duplicate checking logic that is present in the sibling function AB_AddFiles. AB_AddFiles checks each file against the existing folderList to avoid adding duplicates and only passes new files to AB_AddExperimentEntries. AB_AddFolder should implement the same duplicate checking pattern to ensure consistency and avoid processing the same folder multiple times.

Suggested change
string folder
for(folder : folders)
AB_AddElementToSourceList(folder)
endfor
AB_AddExperimentEntries(win, folders)
variable i, index = 0, size
Duplicate/FREE/T folders, newFolders
WAVE/T folderList = GetAnalysisBrowserGUIFolderList()
size = DimSize(folders, ROWS)
for(i = 0; i < size; i += 1)
FindValue/TEXT=folders[i]/TXOP=4 folderList
if(V_Value >= 0)
continue
endif
AB_AddElementToSourceList(folders[i])
newFolders[index] = folders[i]
index += 1
endfor
Redimension/N=(index) newFolders
AB_AddExperimentEntries(win, newFolders)

Copilot uses AI. Check for mistakes.
/// @brief Add folders to the analysis browser
///
/// @param win analysis browser window
/// @param folders text wave with absolute folder paths containing pxps (backslashes need escaping)
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The documentation states "absolute folder paths containing pxps" but the function actually scans for pxp, uxp, and nwb files based on the checkbox settings in the analysis browser, as can be seen in AB_AddExperimentEntries. The documentation should be updated to reflect this, for example: "absolute folder paths containing data files (pxp, uxp, nwb)"

Suggested change
/// @param folders text wave with absolute folder paths containing pxps (backslashes need escaping)
/// @param folders text wave with absolute folder paths containing data files (pxp, uxp, nwb) (backslashes need escaping)

Copilot uses AI. Check for mistakes.
AB_AddExperimentEntries(win, folders)
End

static Function AB_AddElementToSourceList(string entry)
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Making AB_AddElementToSourceList static breaks existing test code in Packages/tests/UTF_HelperFunctions.ipf (line 1221) which calls MIES_AB#AB_AddElementToSourceList. While the new AB_AddFolder function provides a better public API, the test helper function OpenAnalysisBrowser needs to be updated to use the new public API instead of directly calling AB_AddElementToSourceList. Consider keeping AB_AddElementToSourceList public temporarily with a deprecation comment, or ensure all callers are updated in the same change.

Suggested change
static Function AB_AddElementToSourceList(string entry)
/// @deprecated Use AB_AddFolder instead. This function will be made static/private in the future.
Function AB_AddElementToSourceList(string entry)

Copilot uses AI. Check for mistakes.
///
/// @param win analysis browser window
/// @param folders text wave with absolute folder paths containing pxps (backslashes need escaping)
Function AB_AddFolder(string win, WAVE/T folders)
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The function name AB_AddFolder is singular but the parameter is named 'folders' (plural) and the function accepts multiple folders via a wave. For consistency with the sibling function AB_AddFiles which uses plural naming, consider renaming this to AB_AddFolders to better reflect that it can handle multiple folders.

Suggested change
Function AB_AddFolder(string win, WAVE/T folders)
Function AB_AddFolders(string win, WAVE/T folders)

Copilot uses AI. Check for mistakes.
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 this pull request may close these issues.

2 participants