Community
Participate
Working Groups
Created attachment 166638 [details] Patch that adds new MITargetDownload command and modifies CommandFactory. "target-download" is not supported by DSF/GDB. Attached is a patch that adds a new command (MITargetDownload) and modifies CommandFactory. Note: I didn't add the "since" annotations because the versioning of DSF is different.
(In reply to comment #0) > Created an attachment (id=166638) [details] > Patch that adds new MITargetDownload command and modifies CommandFactory. > > "target-download" is not supported by DSF/GDB. > Attached is a patch that adds a new command (MITargetDownload) and modifies > CommandFactory. Note: I didn't add the "since" annotations because the > versioning of DSF is different. Nice. Just some questions. 1- I think you can put the @since 3.0 on MITargetDownload 2- I guess you are not interested in the result of the MITargetDownload? Apparently GDB will return something like ^done,address="0x10004",load-size="9880",transfer-rate="6586",write-rate="429" which does not seem overly interesting to me either... 3- The new API of command factory requires a ICommandControlDMContext while MITargetDownload requires only an IDMContext. What that on purpose? 4- I'm guessing that your setting of Pref->Java->CodeSyte->OrganizeImports has "Number of imports needed for .*" set to something lower than mine. Is there some agreement for CDT? I'm asking because after you commit this change to CommandFactory, if I have to change that class, my eclipse will automatically change all the imports back. My setting was the default of 99 (I think its the default because I never knew about this preference before :-)).
(In reply to comment #1) > Nice. Just some questions. > 1- I think you can put the @since 3.0 on MITargetDownload Will do assuming it is not too late. > 2- I guess you are not interested in the result of the MITargetDownload? > Apparently GDB will return something like > ^done,address="0x10004",load-size="9880",transfer-rate="6586",write-rate="429" > which does not seem overly interesting to me either... I can modify the patch and a new MIInfo, if it's not too late for API changes. In fact this command is a special command: it reports progress. I think we can take advantage of it in the future and display it in the launch configuration progess bar. But it requires a new MICommand type. > 3- The new API of command factory requires a ICommandControlDMContext while > MITargetDownload requires only an IDMContext. What that on purpose? > 4- I'm guessing that your setting of Pref->Java->CodeSyte->OrganizeImports has > "Number of imports needed for .*" set to something lower than mine. Is there > some agreement for CDT? I'm asking because after you commit this change to > CommandFactory, if I have to change that class, my eclipse will automatically > change all the imports back. My setting was the default of 99 (I think its the > default because I never knew about this preference before :-)). Thanks for pointing me to this preference - I wasn't aware of it and don't .* for imports. What is your number now? Mine is default - 99. Is it possible to set it to infinite? :)
(In reply to comment #2) > (In reply to comment #1) > > Nice. Just some questions. > > 1- I think you can put the @since 3.0 on MITargetDownload > > Will do assuming it is not too late. Oh you mean for API freeze? I guess you're supposed to ask the list. From my point-of-view, you are only adding something new, so I'm ok with it. > > 2- I guess you are not interested in the result of the MITargetDownload? > > Apparently GDB will return something like > > ^done,address="0x10004",load-size="9880",transfer-rate="6586",write-rate="429" > > which does not seem overly interesting to me either... > > I can modify the patch and a new MIInfo, if it's not too late for API changes. > In fact this command is a special command: it reports progress. I think we can > take advantage of it in the future and display it in the launch configuration > progess bar. But it requires a new MICommand type. That sounds nice, but complicated :-) I guess it is not necessary for this patch. > > 3- The new API of command factory requires a ICommandControlDMContext while > > MITargetDownload requires only an IDMContext. What that on purpose? > > 4- I'm guessing that your setting of Pref->Java->CodeSyte->OrganizeImports has > > "Number of imports needed for .*" set to something lower than mine. Is there > > some agreement for CDT? I'm asking because after you commit this change to > > CommandFactory, if I have to change that class, my eclipse will automatically > > change all the imports back. My setting was the default of 99 (I think its the > > default because I never knew about this preference before :-)). > > Thanks for pointing me to this preference - I wasn't aware of it and don't .* > for imports. What is your number now? Mine is default - 99. Is it possible to > set it to infinite? :) My number is 99 as well. If yours is too then when doing Ctrl-sht-o in CommandFactory it should remove the .* in the import.
(In reply to comment #2) Oh yeah, what about: > > 3- The new API of command factory requires a ICommandControlDMContext while > > MITargetDownload requires only an IDMContext. Was that on purpose?
(In reply to comment #4) > (In reply to comment #2) > Oh yeah, what about: > > > 3- The new API of command factory requires a ICommandControlDMContext while > > > MITargetDownload requires only an IDMContext. Was that on purpose? Sorry for being sloppy. I have been trying to get the patch in time and simply copied and pasted pieces from different commands. But I am looking now at say MITargetAttach and trying to understand what difference it makes. getCommandControlId of ICommandControlDMContext doesn't seem to get called anywhere. Can you please clarify?
(In reply to comment #5) > > > > 3- The new API of command factory requires a ICommandControlDMContext while > > > > MITargetDownload requires only an IDMContext. Was that on purpose? > > Sorry for being sloppy. I have been trying to get the patch in time and simply > copied and pasted pieces from different commands. But I am looking now at say > MITargetAttach and trying to understand what difference it makes. > getCommandControlId of ICommandControlDMContext doesn't seem to get called > anywhere. Can you please clarify? No actual difference. I personally like choosing the most specific context the code will expect, just as a way to document the API better. When I see a command that wants an IDMContext, it is always difficult to understand which one I should use. If the API requires an IBreakpointDMContext, for example, then it makes it easier on the caller to know what to do. For target-download, I assume we only need to know the ICommandControlDMC (i.e., which gdb), and nothing about a thread, process or anything like that. So ICommandControlDMC seems good. If you had a case where you wanted to accept different things you could use an IDMContext, or you could have multiple constructors (MIDataEvaluateExpression is an example of the latter).
(In reply to comment #6) Thanks, I'll update the patch.
Created attachment 166741 [details] Updated patch.
Changed the argument type of the constructor to ICommandControlDMContext. Added new MITargetDownloadInfo class to process the result. Corrected the since tags.
(In reply to comment #8) > Created an attachment (id=166741) [details] > Updated patch. Nice. I noticed that GDB accepts another parameter to this command: "offset". Just in case you are interested in supporting it. Neither "offset" nor "file" are documented in the GDB manual. (gdb) interpreter-exec mi "-target-download j j" ^error,msg="Invalid download offset:j." (gdb) interpreter-exec mi "-target-download j 2" ^error,msg="j: No such file or directory."
(In reply to comment #10) > (In reply to comment #8) > > Created an attachment (id=166741) [details] [details] > > Updated patch. > Nice. > I noticed that GDB accepts another parameter to this command: "offset". Just > in case you are interested in supporting it. Neither "offset" nor "file" are > documented in the GDB manual. > (gdb) interpreter-exec mi "-target-download j j" > ^error,msg="Invalid download offset:j." > (gdb) interpreter-exec mi "-target-download j 2" > ^error,msg="j: No such file or directory." Don't know, in theory it should have "offset" and probably some others. cc-ing Dan, maybe he can help. Can we go ahead with this? I am worried of missing this milestone.
(In reply to comment #11) > Can we go ahead with this? I am worried of missing this milestone. Yes of course. I'm assuming you are doing those commits.
Committed to the HEAD.
*** cdt cvs genie on behalf of mkhodjai *** Bug 311210 - [commands][cdi] Missing support for "target-download". [+] MITargetDownloadInfo.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/output/MITargetDownloadInfo.java?root=Tools_Project&revision=1.1&view=markup [+] MITargetDownload.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MITargetDownload.java?root=Tools_Project&revision=1.1&view=markup [*] CommandFactory.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/CommandFactory.java?root=Tools_Project&r1=1.6&r2=1.7
*** Bug 237021 has been marked as a duplicate of this bug. ***
