Tomivix / hpa

First release (v0.9.9) is available here: https://github.com/Tomivix/hpa/raw/master/rel/HPA_Studio_0.9.9_%5B576235A3%5D.jar
Other
5 stars 2 forks source link

Added parsing method to Engine.java #7

Closed Tomivix closed 7 years ago

Tomivix commented 7 years ago

Also bulked up some arithmetic operations, added dummy main method and a bunch of comments

Tomivix commented 7 years ago

Waiting for your opinions @Aicedosh and @mrwasp before merging. (Old code is still present in comments, will get removed on merge)

Tomivix commented 7 years ago

Syntax splitting is ready (namely Engine.getPos method). You should take a look at it @Aicedosh , implementing correct syntax highlighting should be possible with that.

Aicedosh commented 7 years ago

Not exactly I wanted.My fault, I should've specified it earlier. I'd rather it accepted a whole text (all lines) and returned a int[][] which: -on index 0 holds all orders/directives in format: [order1StartIndex, order1Length, order2StartIndex, order2Length, ...] -on index 1 all labels (formated in same manner) -on index 2 all arguments -on index 3 all invalid text (e.g. incorrectly formatted line)

I could work with it as is but if you could rework it, that'd be great.

Tomivix commented 7 years ago

Okay I could do that but first I have couple of questions for you @Aicedosh :

  1. What exactly should it return: order with assigned label and its attributes (so it will be matched 1:1:1) or rather just a list of those part (indexes of all orders, indexes of all labels, indexes of all arguments)?
  2. By order/directive you mean just name of command (like 'AR') or the whole line (like 'AR 1,2')?
  3. What should be assigned for orders without label? (only if you picked former answer in question 1.)
  4. By invalid text you mean a whole line? (like for 'ABC : DC INT(10A)' it should store whole line?)
Aicedosh commented 7 years ago
  1. Example array: [order1StartIndex, order1Length, order2StartIndex, order2Length, ...], [label1StartIndex, label1Length, label2StartIndex, label2Length, ...], [argument1StartIndex, argument1Length, ...], [invalidText1StartIndex, invalidText1Length, ...]]. When applying color it doesn't matter which label or argument belongs to which order
  2. As an order I mean the command name(whatever gets marked as an order will be same color/style) 4.If you can specify what user typed wrong (i.e. label contains invalid characters, command name is invalid) then it would be nice to mark only that part (whole label or just invalid characters, invalid command name) as invalid. Alternatively storing whole line will work (whole line will be for example underlined/italic and red or something like that)
Tomivix commented 7 years ago

Well okay, now I know all the details and will start working on that. Two more things @Aicedosh :

Aicedosh commented 7 years ago
  1. separating on '\n' will work
  2. absolute index
Tomivix commented 7 years ago

Okay will proceed with this one. It may take a while though.

Tomivix commented 7 years ago

Splitting method updated and tested. You can start playing with it now @Aicedosh but take note that order in return array is a bit different from your suggestion (to make implementation simpler of course)

Aicedosh commented 7 years ago

It doesn't seem to work for me. I'll play with it tomorrow. For now I'll make a pull request with line numbering in a sec

Tomivix commented 7 years ago

Well, what exactly do you mean by 'doesn't work'? What is the return array?

Aicedosh commented 7 years ago

I don't have time to test that now, all I see is that the colors in code areas are weirdly off

Tomivix commented 7 years ago

Okay, give me more details as soon as you some have time.

Aicedosh commented 7 years ago

So I gave it a little test and:

  1. When you delete whole text, program throws ArrayIndexOutOfBoundsException. Probably caused by returning array of length 1 by split method. If possible always return array of length 4
  2. It'd be nice if you could separate directives and orders split method (right now inserting order in directives panel and vice versa results in valid syntax)
  3. Do you include '\n' chars in indexes? Because it seems like every new line offsets colors by 1: hpa - ss 2
  4. Right now until what is in the line is a completely valid command, whole line highlights in red. It'd be nice if until there is certainly a problem in syntax it assumed it is a label or a command (e.g. if there is no '\n' char behind it and the arguments are not yet inserted - but the command name is valid)
Tomivix commented 7 years ago

Okay I'm back. Will take a look at this but few things @Aicedosh :

  1. Yeah you're right. For text length 0 it returns {{-1,-1}}. Will change it to {{-1,-1},{-1,-1},{-1,-1},{-1,-1}}
  2. Of course it can be done but I can't guess from which part of the GUI the text comes. I must add another parameter to Engine.split and Engine.parse to know that (like true for directives, false for orders). Will that be okay with you?
  3. Will fix that but can you pull your newest version of GUI? My BufferedReader on InputStreamReader can return text with slightly different formatting...
  4. Yep, that's all it does for now. Of course I intended to make it work the way you suggest but I have to come up with some way to do it without a lot of regexes first... (Also sorry for late reply - internet connection issues)
Aicedosh commented 7 years ago
  1. It could return {{},{}, {}, {}} instead. I'm okay with empty arrays as long as they exist (would be even better
  2. The text is retrieved from the corresponding text panels. They can call separate methods ot methods with different params. No difference for me
  3. Already pulled
  4. No rush there, we still got plenty of time
Tomivix commented 7 years ago

Changed default return to {{},{},{},{}}, fixed offset and added mode control (updated your two files as well). Should be working fine for now. Also do you know what's up with @mrwasp ? I want to merge soon but he hasn't commented on any of these commits...

Aicedosh commented 7 years ago

No news from @mrwasp, will check the update later

Tomivix commented 7 years ago

Okay please do. I'm thinking of merging without his approval since we already have conflicts with master.

mrwasp commented 7 years ago

Hi, sorry, I haven't much time these days. You can merge code, I will check and test it probably on Wednesday (when I'll come back to "normal" student life :P).

Tomivix commented 7 years ago

Okay, then I will be merging this branch to master. Still waiting for yours and @mrwasp opinion though.