Commit 4de3cefaa138c4610cb4455ac5c5535e599b6a8f
1 parent
1790bed6
Add quotes to sky region names (#3280)
Showing
6 changed files
with
89 additions
and
18 deletions
ChangeLog
NEWS
| ... | ... | @@ -86,7 +86,10 @@ None |
| 86 | 86 | |
| 87 | 87 | 12. Sky module |
| 88 | 88 | -------------- |
| 89 | -None | |
| 89 | +The region files written by GSkyRegions were not readable by ds9 when region | |
| 90 | +names were specified. It turned out that region names have to be put in quotes, | |
| 91 | +either ", ' or {}. I modified the GSkyRegionCircle::read() and write() methods | |
| 92 | +to support this logic, where {} are used in GSkyRegionCircle::write() (#3280). | |
| 90 | 93 | |
| 91 | 94 | |
| 92 | 95 | 13. Support module | ... | ... |
doc/source/admin/release_history/1.7.1.rst
| ... | ... | @@ -10,5 +10,7 @@ GammaLib 1.7.1 is a bug fix release for GammaLib release 1.7. |
| 10 | 10 | |
| 11 | 11 | The following bugs were fixed: |
| 12 | 12 | |
| 13 | +* [`3280 <https://cta-redmine.irap.omp.eu/issues/3280>`_] - | |
| 14 | + Add quotes to sky region names | |
| 13 | 15 | * [`3279 <https://cta-redmine.irap.omp.eu/issues/3279>`_] - |
| 14 | 16 | Fix broken power law flux and eflux computation bug | ... | ... |
src/sky/GSkyRegionCircle.cpp
| ... | ... | @@ -303,8 +303,8 @@ void GSkyRegionCircle::read(const std::string& line) |
| 303 | 303 | std::string system = gammalib::split(region_def, ";")[0]; |
| 304 | 304 | |
| 305 | 305 | // Get the substring of the important values |
| 306 | - unsigned pos = region_def.find("circle("); | |
| 307 | - unsigned end = region_def.find(")"); | |
| 306 | + size_t pos = region_def.find("circle("); | |
| 307 | + size_t end = region_def.find(")"); | |
| 308 | 308 | std::string circlestring = region_def.substr(pos+7, end); |
| 309 | 309 | circlestring.erase(circlestring.find(")"), 1); |
| 310 | 310 | |
| ... | ... | @@ -355,21 +355,68 @@ void GSkyRegionCircle::read(const std::string& line) |
| 355 | 355 | // Compute solid angle |
| 356 | 356 | compute_solid_angle(); |
| 357 | 357 | |
| 358 | - // Check if there is a given name for the region and set it | |
| 359 | - std::vector<std::string>comments = gammalib::split(comment, " "); | |
| 360 | - for (int i = 0; i < comments.size(); i++) { | |
| 361 | - if (gammalib::contains(comments[i], "text")) { | |
| 362 | - std::vector<std::string> attributes = gammalib::split(comments[i], "="); | |
| 363 | - if (attributes.size() < 2) { | |
| 364 | - std::string msg = | |
| 365 | - "Invalid character sequence encountered in provided" | |
| 366 | - " string \""+line+"\".\n" | |
| 367 | - "An attribute of the type \"text=Name\" is expected."; | |
| 368 | - throw GException::invalid_value(G_READ, msg); | |
| 369 | - } | |
| 370 | - m_name = attributes[1]; | |
| 371 | - } | |
| 372 | - } | |
| 358 | + // Check if there is a given name for the region and set it. Strings may | |
| 359 | + // be quoted with " or ' or {} | |
| 360 | + pos = comment.find("text"); | |
| 361 | + if (pos != std::string::npos) { | |
| 362 | + pos = comment.find("=", pos+4); | |
| 363 | + if (pos != std::string::npos) { | |
| 364 | + | |
| 365 | + // Set possible quotes | |
| 366 | + const std::string quotes_open = "{'\""; | |
| 367 | + const std::string quotes_close = "}'\""; | |
| 368 | + | |
| 369 | + // Set quotes found flag | |
| 370 | + bool found = false; | |
| 371 | + | |
| 372 | + // Search for possible quotes and extract name | |
| 373 | + for (int i = 0; i < quotes_open.size(); ++i) { | |
| 374 | + std::string quote_open = quotes_open.substr(i,1); | |
| 375 | + std::string quote_close = quotes_close.substr(i,1); | |
| 376 | + pos = comment.find(quote_open, pos+1); | |
| 377 | + if (pos != std::string::npos) { | |
| 378 | + end = comment.find(quote_close, pos+1); | |
| 379 | + if (end != std::string::npos) { | |
| 380 | + int length = end-pos-1; | |
| 381 | + if (length > 0) { | |
| 382 | + m_name = comment.substr(pos+1, length); | |
| 383 | + } | |
| 384 | + else { | |
| 385 | + m_name = ""; | |
| 386 | + } | |
| 387 | + found = true; | |
| 388 | + } | |
| 389 | + else { | |
| 390 | + std::string msg = "Missing "+quote_open+" following "+ | |
| 391 | + quote_close+" encountered after " | |
| 392 | + "text attribute. Please add closing " | |
| 393 | + "quotes to text="+quote_open+"Name"+ | |
| 394 | + quote_close+" attribute."; | |
| 395 | + throw GException::invalid_value(G_READ, msg); | |
| 396 | + } | |
| 397 | + } | |
| 398 | + } // endfor: looped over possible quotes | |
| 399 | + | |
| 400 | + // Throw an exception if no quotes were found | |
| 401 | + if (!found) { | |
| 402 | + std::string msg = "Missing quotes \" or ' or {} following " | |
| 403 | + "text attribute. Please specify quotes " | |
| 404 | + "around the names of the text, for example " | |
| 405 | + "text={Name}."; | |
| 406 | + throw GException::invalid_value(G_READ, msg); | |
| 407 | + } | |
| 408 | + | |
| 409 | + } // endif: = character found | |
| 410 | + | |
| 411 | + // Signal that no = character was found after text | |
| 412 | + else { | |
| 413 | + std::string msg = "Missing = following text attribute. Please " | |
| 414 | + "specify the value for a text attribute, " | |
| 415 | + "for example text={Name}."; | |
| 416 | + throw GException::invalid_value(G_READ, msg); | |
| 417 | + } | |
| 418 | + | |
| 419 | + } // endif: text attribute found | |
| 373 | 420 | |
| 374 | 421 | // Return |
| 375 | 422 | return; |
| ... | ... | @@ -401,7 +448,9 @@ std::string GSkyRegionCircle::write(void) const |
| 401 | 448 | // Optionally add region name |
| 402 | 449 | if (m_name.length() > 0) { |
| 403 | 450 | result.append(" # text="); |
| 451 | + result.append("{"); | |
| 404 | 452 | result.append(m_name); |
| 453 | + result.append("}"); | |
| 405 | 454 | } |
| 406 | 455 | |
| 407 | 456 | // Return string | ... | ... |
test/data/test_circle_region.reg
| ... | ... | @@ -2,3 +2,7 @@ |
| 2 | 2 | global color=green dashlist=8 3 width=1 font="helvetica 10 normal roman" select=1 highlite=1 dash=0 fixed=0 edit=1 move=1 delete=1 include=1 source=1 |
| 3 | 3 | fk5 |
| 4 | 4 | circle(0.1, -35.6, 10.0) |
| 5 | +circle(0.1, -35.6, 10.0) # text={} | |
| 6 | +circle(0.1, -35.6, 10.0) # text={Test 1} | |
| 7 | +circle(0.1, -35.6, 10.0) # text='Test 2' | |
| 8 | +circle(0.1, -35.6, 10.0) # text="Test 3" | ... | ... |
test/test_GSky.cpp
| ... | ... | @@ -1594,11 +1594,17 @@ void TestGSky::test_GSkyMap_io(void) |
| 1594 | 1594 | } |
| 1595 | 1595 | |
| 1596 | 1596 | // Check if region was loaded correctly |
| 1597 | + test_value(regions.size(), 5, "5 sky regions expected"); | |
| 1597 | 1598 | GSkyRegionCircle* circle = dynamic_cast<GSkyRegionCircle*>(regions[0]); |
| 1598 | 1599 | test_assert(circle->type() == "Circle", "Region is not a circle"); |
| 1599 | 1600 | test_value(circle->radius(), 10.0, 1.0e-10); |
| 1600 | 1601 | test_value(circle->ra(), 0.1, 1.0e-10); |
| 1601 | 1602 | test_value(circle->dec(), -35.6, 1.0e-10); |
| 1603 | + test_value(regions[0]->name(), "", "No name for first sky region"); | |
| 1604 | + test_value(regions[1]->name(), "", "Empty name for second sky region"); | |
| 1605 | + test_value(regions[2]->name(), "Test 1", "Name \"Test 1\" for third sky region"); | |
| 1606 | + test_value(regions[3]->name(), "Test 2", "Name \"Test 2\" for fourth sky region"); | |
| 1607 | + test_value(regions[4]->name(), "Test 3", "Name \"Test 3\" for fifth sky region"); | |
| 1602 | 1608 | |
| 1603 | 1609 | // Test regions saving |
| 1604 | 1610 | test_try("Test regions saving"); |
| ... | ... | @@ -1621,11 +1627,17 @@ void TestGSky::test_GSkyMap_io(void) |
| 1621 | 1627 | } |
| 1622 | 1628 | |
| 1623 | 1629 | // Check if region was loaded correctly |
| 1630 | + test_value(regions.size(), 5, "5 sky regions expected"); | |
| 1624 | 1631 | circle = dynamic_cast<GSkyRegionCircle*>(regions[0]); |
| 1625 | 1632 | test_assert(circle->type() == "Circle", "Region is not a circle"); |
| 1626 | 1633 | test_value(circle->radius(), 10.0, 1.0e-10); |
| 1627 | 1634 | test_value(circle->ra(), 0.1, 1.0e-10); |
| 1628 | 1635 | test_value(circle->dec(), -35.6, 1.0e-10); |
| 1636 | + test_value(regions[0]->name(), "", "No name for first sky region"); | |
| 1637 | + test_value(regions[1]->name(), "", "Empty name for second sky region"); | |
| 1638 | + test_value(regions[2]->name(), "Test 1", "Name \"Test 1\" for third sky region"); | |
| 1639 | + test_value(regions[3]->name(), "Test 2", "Name \"Test 2\" for fourth sky region"); | |
| 1640 | + test_value(regions[4]->name(), "Test 3", "Name \"Test 3\" for fifth sky region"); | |
| 1629 | 1641 | |
| 1630 | 1642 | // Exit test |
| 1631 | 1643 | return; | ... | ... |