Added a function to add backslash before a space #541

MichalKesl · 2022-08-10T12:02:44Z

If path has spaces the function will modify the name and add backslash before the space. a local test was preformed, the solution works.

fixes #535

rwest · 2022-08-10T19:05:44Z

If you do want to just replace all the with \ then you could consider folder_name.replace(' ','\ ') instead of your split and recombine routine.

In [3]: folder_name = 'foo bar  baz'

In [4]: folder_name.replace(' ','\ ')
Out[4]: 'foo\\ bar\\ \\ baz'

But there may be a better fix. This won't escape anything except spaces.

Most functions don't need the path escaping. The few that do include os.system(), but it might be safer to replace those calls with subprocess and then you avoid the need. But maybe it's for doing something over SSH where shell escaping is not avoidable. In that case, consider shlex.quote()

Return a shell-escaped version of the string s. The returned value is a string that can safely be used as one token in a shell command line, for cases where you cannot use a list.

alongd

Thanks, please see some comments

alongd · 2022-11-23T07:07:57Z

arc/common.py

+    arc_r_symbols = [atom.element.symbol for atom in
+                     chain(*tuple(arc_reaction.r_species[i].mol.atoms for i in range(num_rs)))]
+    arc_p_symbols = [atom.element.symbol for atom in
+                     chain(*tuple(arc_reaction.p_species[i].mol.atoms for i in range(num_ps)))]


All the modifications to the file up to this point are style changes. If you think they are necessary, please add them as a different commit. I would recommend removing them, the previous version is "OK", I think

alongd · 2022-11-23T07:08:15Z

arc/common.py

+        new_folder_name (str): modified folder name.
+
+    """
+    folder_name.split(" ")


this line has no effect (we don't store the returned value)

alongd · 2022-11-23T07:08:58Z

arc/common.py

+    """
+    folder_name.split(" ")
+    new_folder_name = ""
+    for i in folder_name.split(" "):


minor: I would recommend changing i to a different name, i is usually an integer iterator. maybe use split?

You can do either folder_name.split() or folder_name.split(" "), they are the same (the first one is shorter or "cleaner")

alongd · 2022-11-23T07:10:56Z

arc/common.py

+    folder_name.split(" ")
+    new_folder_name = ""
+    for i in folder_name.split(" "):
+        new_folder_name += i + "\ "


If you'd like, you can achieve this in a single line instead of the loop. Try:
new_folder_name = '\ '.join(folder_name.split())

codecov · 2023-05-29T11:33:12Z

Codecov Report

Merging #541 (df9dff7) into main (c440aab) will increase coverage by 0.01%.
Report is 3 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #541      +/-   ##
==========================================
+ Coverage   73.18%   73.20%   +0.01%     
==========================================
  Files          99       99              
  Lines       26522    26534      +12     
  Branches     5546     5546              
==========================================
+ Hits        19409    19423      +14     
+ Misses       5772     5771       -1     
+ Partials     1341     1340       -1

Flag	Coverage Δ
unittests	`73.20% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
arc/common.py	`82.60% <100.00%> (+0.33%)`	⬆️
arc/common_test.py	`99.85% <100.00%> (+<0.01%)`	⬆️
arc/main.py	`51.03% <100.00%> (+0.07%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

alongd

Thanks @MichalKesl, looks good!
I added some comments, please take a look

arc/common_test.py

alongd · 2023-05-30T05:15:48Z

arc/common_test.py

+        self.assertTrue(common.fill_in_the_blanks(ex1), "michalkfir")
+        self.assertTrue(common.fill_in_the_blanks(ex2), "michal\\ kfir")
+        self.assertTrue(common.fill_in_the_blanks(ex3), "mich\\ al\\ kfir")
+        self.assertTrue(common.fill_in_the_blanks(ex4), "michal\\ \\ kfir")


why do we have two slashes in the tests?

If we do michal kfir.replace(" ", "\ ") the string changes to michal\\ kfir, because \ is an operator (like \n).

alongd · 2023-08-19T23:57:08Z

@MichalKesl, when you get a chance, can we finish this PR?

calvinp0 · 2023-11-14T10:03:49Z

@MichalKesl is this PR relevant?

alongd · 2025-08-26T14:36:13Z

@calvinp0, I revived it, could you please have a look and merge?

Copilot

Pull Request Overview

This PR adds a utility function to handle file paths containing spaces by escaping them with backslashes. The function fill_in_the_blanks is implemented to modify folder names by adding backslashes before spaces, which is commonly needed in Unix-like systems for shell compatibility.

Added fill_in_the_blanks function to escape spaces in file paths
Integrated the function into the ARC class constructor to automatically handle project directories with spaces
Added comprehensive test coverage for various space-containing scenarios

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
arc/common.py	Implements the `fill_in_the_blanks` function to escape spaces in folder names
arc/main.py	Imports and applies the new function to project directory paths
arc/common_test.py	Adds unit tests covering various space-escaping scenarios

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-27T06:30:32Z

arc/common.py

+    Returns:
+        str: Modified folder name.
+    """
+    return folder_name.replace(" ", "\ ")


The backslash escaping is incorrect. In Python string literals, you need to escape the backslash itself. This should be folder_name.replace(' ', '\\ ') or use a raw string folder_name.replace(' ', r'\ ').

Suggested change

return folder_name.replace(" ", "\ ")

return folder_name.replace(" ", "\\ ")

Copilot · 2025-08-27T06:30:33Z

arc/common.py

+def fill_in_the_blanks(folder_name: str):
+    """
+    Adding backslashes before spaces in the folder name.
+
+    Args:
+        folder_name (str): The string directory we want to modify.
+
+    Returns:
+        str: Modified folder name.
+    """
+    return folder_name.replace(" ", "\ ")


[nitpick] The docstring should clarify that this function is for shell compatibility and mention that it works on any path, not just folder names. Also, the parameter description could be more precise: 'The file or directory path to escape'.

Suggested change

def fill_in_the_blanks(folder_name: str):

"""

Adding backslashes before spaces in the folder name.

Args:

folder_name (str): The string directory we want to modify.

Returns:

str: Modified folder name.

"""

return folder_name.replace(" ", "\ ")

def fill_in_the_blanks(path: str):

"""

Escape spaces in a file or directory path by adding backslashes before spaces for shell compatibility.

This function is useful when constructing shell commands that use paths containing spaces.

Args:

path (str): The file or directory path to escape.

Returns:

str: The modified path string with spaces escaped.

"""

return path.replace(" ", "\ ")

calvinp0

I am not really sure about this function - if we add the '\ ' to the space then I am not sure if it cause issues with functions like os.path.exists and os.makedirs and calling subprocesses.

calvinp0 · 2025-08-27T06:46:08Z

I would like to see tests for those specific functions with this function being utilized

github-actions bot added the Module: Main label Aug 10, 2022

MichalKesl added the Do not merge label Aug 10, 2022

alongd reviewed Nov 23, 2022

View reviewed changes

MichalKesl force-pushed the folder_rename branch from bf2c038 to d48f504 Compare May 29, 2023 10:32

MichalKesl changed the title ~~Issue #535 solved~~ Added a function to add backslash before a space May 29, 2023

MichalKesl requested a review from alongd May 29, 2023 11:43

alongd reviewed May 30, 2023

View reviewed changes

MichalKesl force-pushed the folder_rename branch from d48f504 to df9dff7 Compare August 20, 2023 12:58

alongd force-pushed the folder_rename branch from df9dff7 to bea2e97 Compare August 25, 2025 16:42

MichalKesl added 3 commits August 26, 2025 21:14

Added fill_in_the_blanks() for treating whitespace in folder names

2e947ab

Tests: fill_in_the_blanks()

d18edb9

Use fill_in_the_blanks() for project_directory in main

ba90281

alongd force-pushed the folder_rename branch from bea2e97 to ba90281 Compare August 26, 2025 18:14

calvinp0 requested a review from Copilot August 27, 2025 06:29

Copilot AI reviewed Aug 27, 2025

View reviewed changes

calvinp0 requested changes Aug 27, 2025

View reviewed changes

	return folder_name.replace(" ", "\ ")
	return folder_name.replace(" ", "\\ ")

Added a function to add backslash before a space #541

Are you sure you want to change the base?

Added a function to add backslash before a space #541

Conversation

MichalKesl commented Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rwest commented Aug 10, 2022

Uh oh!

alongd left a comment

Choose a reason for hiding this comment

Uh oh!

alongd Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

alongd Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

alongd Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

alongd Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

alongd Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alongd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alongd May 30, 2023

Choose a reason for hiding this comment

Uh oh!

MichalKesl Aug 20, 2023

Choose a reason for hiding this comment

Uh oh!

alongd commented Aug 19, 2023

Uh oh!

calvinp0 commented Nov 14, 2023

Uh oh!

alongd commented Aug 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

calvinp0 left a comment

Choose a reason for hiding this comment

Uh oh!

calvinp0 commented Aug 27, 2025

Uh oh!

Uh oh!

MichalKesl commented Aug 10, 2022 •

edited

Loading

codecov bot commented May 29, 2023 •

edited

Loading