Skip to content

'sed -i' need postfix on mac #3312

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.

Sign up for GitHub

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

Closed
DaanHoogland wants to merge 2 commits into from
Closed

'sed -i' need postfix on mac #3312

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
067a591
'sed -i' need postfix on mac
DaanHoogland May 2, 2019
025851c
oops, commented code :(
DaanHoogland May 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions tools/apidoc/build-apidoc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@


# cloud-build-api-doc.sh -- builds api documentation.
#set -x
#set -u

ACS_RELEASE="$1"
shift
TARGETJARDIR="$1"
Expand Down Expand Up @@ -61,8 +60,8 @@ set -e
cp "$thisdir"/*.java .
cp "$thisdir"/*.xsl .
sed -e 's,%API_HEADER%,All APIs,g' "$thisdir/generatetoc_header.xsl" >generatetoc.xsl
sed -i "s/%ACS_RELEASE%/${ACS_RELEASE}/g" generatetoc.xsl
sed -i "s/%ACS_RELEASE%/${ACS_RELEASE}/g" generatecommands.xsl
sed -i .bak "s/%ACS_RELEASE%/${ACS_RELEASE}/g" generatetoc.xsl

@rafaelweingartner rafaelweingartner May 2, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if we execute the commands at line 63 and 64 like the one in 62? Or, maybe the other way around.

I mean, they do the same thing, but with different approaches. Therefore, it might be interesting to maintain some consistency here.

@DaanHoogland DaanHoogland May 2, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rafaelweingartner, not entirely sure what you mean but 62 an 63 can certainly be combined. not sure how 64 can, it is touching a different file.

@rafaelweingartner rafaelweingartner May 2, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I meant the following:
All of the commands are replacing "place holders" in the same file. However, two of them are doing in line (via -i flag), whereas line 62 is replacing the entire file (not using the -i flag and redirecting the output of the sed to a file with the same name). I do not mind if we do in different commands the replace, but we could at least try to use the same approach in all of them.

By approach here I mean the method, either inline or not.

@DaanHoogland DaanHoogland May 2, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think you are misreading the code,
line 62 redirects to a new file, from "$thisdir/generatetoc_header.xsl" to generatetoc.xsl
then 63 operates on the resulting generatetoc.xsl
but 64 is operating on generatecommands.xsl

I don't think shell operations would allow redecting to input, it would empty the file before openening it for read if you tried that.

@GabrielBrascher GabrielBrascher May 17, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@DaanHoogland @rafaelweingartner A (ugly) solution is running the command with the -e and move the new file to the old one:
sed -e "s/%ACS_RELEASE%/${ACS_RELEASE}/g" newGeneratetoc.xsl && mv newGeneratetoc.xsl generatetoc.xsl;

@GabrielBrascher GabrielBrascher May 17, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@DaanHoogland on linux the -i must be appended with the .bak (-i.bak)

-i[SUFFIX], --in-place[=SUFFIX]
              edit files in place (makes backup if SUFFIX supplied)

sed -i .bak "s/%ACS_RELEASE%/${ACS_RELEASE}/g" generatecommands.xsl

PLATFORM=`uname -s`
if [[ "$PLATFORM" =~ .*WIN.* ]]
Expand Down