Mon 15 Oct 2012 05:22:48 PM UTC, comment #5:
Thanks, I have pushed and merged your commmit, with some modification:
http://hg.savannah.gnu.org/hgweb/octave/graph/73d23a6e7bf3
I edited your commit message, since apparently you didn't pick this up when amending it. Note that this means the commit in Savannah is different from (has a different hash than) the one in your local repo. You may want to pull from Savannnah and update (hg pull --update), and get rid of your version of the patch (hg strip -r 82f02538f90).
One final thing, you inadvertently put two patches into the same patch file, due to the following bug:
http://bz.selenic.com/show_bug.cgi?id=3652
|
Mon 15 Oct 2012 03:41:47 PM UTC, comment #4:
Ok, thanks for the various tips. I hadn't actually read the wiki page you were talking about, just the Octave manual version.
I have created a new changeset, but I may have screwed this up.
In it I just removed both file and state, I also fixed a bug I'd created for the two files case. I also removed the whitespace.
(file #26772)
|
Mon 15 Oct 2012 02:41:50 PM UTC, comment #3:
> Most of the changes shown in the changeset are due to me putting a lot
> of the original code inside an if statement, and therefore adding
> spaces at the start of the lines.
Ah, I see. I just looked at your change ignoring whitespace, and it
was much more readable.
> In the original code you could supply either one argument (a file
> name) or two arguments (an editor state to be changed and the new
> value of that state). In the second case then the first argument
> (named file) was already no longer actually a file name anyway.
Yes, what I mean is that you should remove the "state" from the
function definition entirely and instead have
and deal with the ensuing logic in the function body.
> I could add more comments to explain this if you like.
Yes, please.
> I am very unfamiliar with hg, is the exact sequence of things I need
> to do the following?
> hg up -r 30c8b5d22e
> make my edits to edit.m
> hg commit edit.m --amend -m"new commit msg"
Yes, minus the -m. Then hg will open a new editor window where you can
edit the commit message that's already there.
One further hg tip, if you're using an OS with a functional terminal
(i.e. not Windows), try adding (possibly a variation of) the following
lines to your .hgrc:
When you look at diffs with hg this way, they will be coloured, and
you will see that your proposed change has trailing whitespace. Remove
it, please. ;-)
> I had read the guidelines, but misunderstood them, edit.m does
> contain multiple functions (it has subfunctions), so I though I
> would need to be specific from what I read.
The commit message guidelines there aren't quite set in stone, but I
would have written the commit message like
|
Mon 15 Oct 2012 02:20:55 PM UTC, comment #2:
Most of the changes shown in the changeset are due to me putting a lot of the original code inside an if statement, and therefore adding spaces at the start of the lines.
In the original code you could supply either one argument (a file name) or two arguments (an editor state to be changed and the new value of that state). In the second case then the first argument (named file) was already no longer actually a file name anyway.
With my changes you have the following options:
one arg:
either a string, or cell array of strings, each containing a file name to be opened by the editor.
two arg:
either a state name and new state (as in original)
OR
two strings each containing file names to be opened by the editor
more args:
multiple strings, each a file to be opened by the editor
depending on the inputs, I create a cell array of strings containing the list of files to be opened. If there are more than one in this list I call edit.m on each one.
If edit.m is called with a single string, the length of the list is one and the original behaviour is used instead.
I could add more comments to explain this if you like. I am very unfamiliar with hg, is the exact sequence of things I need to do the following?
hg up -r 30c8b5d22e
make my edits to edit.m
hg commit edit.m --amend -m"new commit msg"
I had read the guidelines, but misunderstood them, edit.m does contain multiple functions (it has subfunctions), so I though I would need to be specific from what I read.
|
Mon 15 Oct 2012 12:50:10 PM UTC, comment #1:
Thanks for the patch.
I find this patch a little difficult to read. Are all of these changes really necessary or are there unrelated changes here? I think the way arguments are parsed is rather strange. If the state argument sometimes isn't state at all, then it should be removed from the calling form and absorbed in varargin.
As to the commit message, please consult the following:
http://wiki.octave.org/Commit_message_guidelines
The only required change should be to change "edit/edit: [...]" to "edit.m: [...]".
If you have a recent enough hg version (2.2, I think), I recommend you update to your commit, "hg up -r 30c8b5d22e", make your changes, and then do "hg commit --amend" in order to amend this commit.
|