Code review comment for lp://qastaging/~bryanfritt/compiz/compiz-decorator_script-edit_1192376

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

> COMPIZ_BIN_PATH=$(cd $(dirname "$0"); vs COMPIZ_BIN_PATH=`dirname "$0"`
> The first always gives the full path; The second changes based on how it's
> called. Put it in a script and it'll change based on how the script is called.
> Like is it called via a relative path, or via a full path? I suppose for this
> script this line could be done either way, although I haven't tested the
> script with the second.
>
> you can try it out...
> put this in a file and run different ways
>
> # a quickly made example...
> # with this code in /opt/CompizBZR/bin/pathTest.sh
>
> THIS_PATH_1=$(cd $(dirname "$0"); pwd)
> echo $THIS_PATH_1
>
> THIS_PATH_2=`dirname "$0"`
> echo $THIS_PATH_2
>
> # results:
> # called from /opt/CompizBZR/ with /opt/CompizBZR/bin/pathTests.sh
> # /opt/CompizBZR/bin
> # /opt/CompizBZR/bin
>
> # also called from /opt/CompizBZR/ with ./bin/pathTests.sh
> # /opt/CompizBZR/bin
> # ./bin
>
> # called from ~/ with ../../opt/CompizBZR/bin/pathTests.sh
> # /opt/CompizBZR/bin
> # ../../opt/CompizBZR/bin
>
> # called from /opt/CompizBZR/bin/ with ./pathTests.sh
> # ./pathTests.sh
> # /opt/CompizBZR/bin

Ah okay, what I might suggest is readlink -e, eg

readlink -e "$0"

That works regardless of the path, and it also resolves symlinks too.

Changing the pwd is a bit of a kludge and can get messy if the script crashes or gets killed in-between.

>
> "There's lots of commented out code in the review version."
> This was my first time messing with bzr, and Compiz submissions. I'm guessing
> I hit submit for review too soon?, and guessing it doesn't update which code
> to review? and so you might reviewed an earlier version?
> (And I should resubmit again after changing tabs to 4 spaces?)

It will automatically update. Resubmit if you want - it just sends me another email notification that's all :)

>
> The one I should have submitted should have been done after 3751 `chmod +x`,
> the last thing I've done with this code. I hadn't thought of any changes to do
> with it since then, and it's working for me, so I guess I'm done with this set
> of edits, except for now messing with what you said, the using four spaces in
> place of a tab.
>
> "These changes are inconsistent with the coding style. Use 8-wide tabs for two
> indents and 4-spaces for one indent, or, if the language doesn't support it,
> use 4-spaces for indents."
> I didn't know about the coding style. Where can I read it?
> Should be an easy change. No problem. It'll actually be easier to do copy
> paste code type testing on to the shell with spaces instead of tab. (tab
> characters cause 'tab completions' to activate during copy paste, which makes
> things look weird...)
>

« Back to merge proposal