bugGNU Octave - Bugs: bug #37573, edit.m cannot open multiple file...

 
 

bug #37573: edit.m cannot open multiple file names

Submitter:  Richard <crobar>
Submitted:  Mon 15 Oct 2012 09:12:36 AM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Richard Crozier Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 15 Oct 2012 05:28:58 PM UTC, comment #6: 

Hooray, how exciting!

Thanks for fixing the patch, I need to read the hg book and get some practice.

I actually noticed there were two patches but I though it was because I was patching on top of a patch or something.

Richard <crobar>
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

Jordi Gutiérrez Hermoso <jordigh>
Group Member
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)

Richard <crobar>
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


function ret = edit (file, varargin)


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:


[extensions]
color =
pager =

[pager]
pager = LESS='FSRX' less
attend = tags, help, annotate, cat, diff, export, glog, log, qdiff, status, outgoing, incoming

[color]
status.modified = magenta bold
status.added = green bold
status.removed = red bold
status.deleted = cyan bold
status.unknown = black  bold
status.ignored = black bold


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


open multiple files with edit (bug #37573)

* edit.m (edit): Check args, [etc...]


Jordi Gutiérrez Hermoso <jordigh>
Group Member
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.

Richard <crobar>
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.

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Mon 15 Oct 2012 09:12:36 AM UTC, original submission:  

In matlab, edit can be called with several strings representing file names and open several files with a single call.

The attached changeset adds this functionality to Octave's edit.m.

I have never produced an hg changeset before.

Richard <crobar>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #26772:  edit_mult_files.patch added by crobar (46KiB - text/x-patch)
file #26763:  edit.diff added by crobar (20KiB - text/x-patch - edit.m changeset)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jordigh (Posted a comment)
  • -email is unavailable- added by crobar (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-10-15 jordigh StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2012-10-15 crobar Attached File- Added edit_mult_files.patch, #26772
    2012-10-15 jordigh StatusNone Patch Submitted
    2012-10-15 crobar Attached File- Added edit.diff, #26763

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code